From bd52d565e17fcf321eb3f47e81127abf8ae11cd1 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 10:28:48 -0700 Subject: [PATCH 01/22] feat(bundle): record what an import wrote, so registering the folder back can tell an edit provenance.json, schema 2, now also holds: - the task map, each task file with the store's task id and version, and the eval's; - a fingerprint of the configured document store's section, which tells one store from another without naming where either is; - each file the import wrote, as its content hash (+x when executable). The pinned store references move from "store" to "pinned". Nothing reads the file yet. The file hash the ledger computes moves to _fs, so both use one. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_fs.py | 13 +++++-- src/agent_env/bundle/_import_report.py | 42 ++++++++++++++++++++--- src/agent_env/bundle/importer.py | 2 +- src/agent_env/bundle/ledger.py | 11 ++---- tst/unit/bundle/importer_entities_test.py | 33 +++++++++++++++++- tst/unit/bundle/importer_test.py | 6 ++++ 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/agent_env/bundle/_fs.py b/src/agent_env/bundle/_fs.py index 37d3e890b..738fbe8b6 100644 --- a/src/agent_env/bundle/_fs.py +++ b/src/agent_env/bundle/_fs.py @@ -1,9 +1,10 @@ """Filesystem plumbing for the bundle parser: a path spelled as the filesystem stores it, the user's -home, a regular file read without surprises, names compared without case, and paths rendered for -messages.""" +home, a regular file read without surprises, a file's content hash, names compared without case, and paths rendered +for messages.""" from __future__ import annotations +import hashlib import os import stat import sys @@ -96,6 +97,14 @@ def read_regular(path: Path) -> bytes: return data +def file_sha256(path: Path) -> str: + digest = hashlib.sha256() + with open(path, "rb") as file: + for chunk in iter(lambda: file.read(1 << 20), b""): + digest.update(chunk) + return "sha256:" + digest.hexdigest() + + def fold(name: str) -> str: """The canonical caseless form: casefold output isn't always NFC, so normalize both sides.""" return unicodedata.normalize("NFC", unicodedata.normalize("NFC", name).casefold()) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index 108c2a79b..2d6fe263c 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -3,13 +3,17 @@ from __future__ import annotations +import hashlib import json import posixpath +import unicodedata from dataclasses import dataclass from datetime import datetime, timezone from importlib.metadata import version as package_version from pathlib import Path +from agent_env.bundle._fs import file_sha256 +from agent_env.config import get_config from agent_env.config.model import ENV_API_KEY, ENV_BASE_URL from agent_env.entity.refs import EntityKind from agent_env.store.image_store.oci_registry_credentials import registry_host_from_ref @@ -17,8 +21,10 @@ from agent_env.utils.docker.lock import IN_IMAGE, Locked from agent_env.utils.docker.prune import Pruned -# The file an import records what it wrote in, and from where, for publishing the folder back. +# The file an import records what it wrote in, and from where, for registering the folder back. PROVENANCE = "provenance.json" +# Bumped when what the file records changes. +PROVENANCE_SCHEMA = 2 @dataclass(frozen=True) @@ -155,22 +161,48 @@ def size_text(size: int) -> str: return f"{size} B" if size < 1_000 else f"{size / 1_000:.1f} KB" if size < 1_000_000 else f"{size / 1_000_000:.1f} MB" -def provenance(source: dict, imported: BundleImport) -> str: - """What the folder was imported from and what it holds, by store id and version: no store's locations.""" +def provenance(source: dict, imported: BundleImport, root: Path) -> str: + """What the folder was imported from and what it holds, by store id and version, and each file written at ``root``, + so an edit can be told from what the store holds: no store's locations, only a fingerprint of the store's config.""" return json.dumps({ + "schema": PROVENANCE_SCHEMA, "agent_env": package_version("agentenv-framework"), "imported_at": datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), "from": source, + "store": store_fingerprint(), + "tasks": [{"file": task.path, "id": task.id, "version": task.version} for task in imported.tasks], + "eval": {"file": imported.eval.path, "id": imported.eval.id, "version": imported.eval.version} + if imported.eval else None, "written": [{"folder": entity.folder, "kind": entity.kind.value, "type": entity.type, "id": entity.id, "version": entity.version} for entity in imported.written], "images": [{"folder": image.folder, "key": image.key, "id": image.artifact, "version": image.version, "dockerfile": image.pruned.dockerfile, "dockerfile_found": image.found} for image in imported.images], - "store": [{"where": pin.where, "kind": pin.kind.value, "id": pin.id, "version": pin.version, "why": pin.why} - for pin in imported.pinned], + "pinned": [{"where": pin.where, "kind": pin.kind.value, "id": pin.id, "version": pin.version, "why": pin.why} + for pin in imported.pinned], + "files": written_files(root), }, indent=2, ensure_ascii=False) + "\n" +def store_fingerprint() -> str: + """A hash of the configured document store's section, which tells one store from another without naming where + either is.""" + section = get_config().trace_section("document").value + text = json.dumps(section, sort_keys=True, separators=(",", ":"), default=str) + return "sha256:" + hashlib.sha256(text.encode()).hexdigest() + + +def written_files(root: Path) -> dict[str, str]: + """Each file under ``root`` but its README and provenance, by its path in NFC, as its content hash, ``+x`` after it + when it's executable.""" + files = {} + for path in sorted(root.rglob("*")): + relative = unicodedata.normalize("NFC", path.relative_to(root).as_posix()) + if path.is_file() and relative not in ("README.md", PROVENANCE): + files[relative] = file_sha256(path) + ("+x" if path.stat().st_mode & 0o111 else "") + return files + + def readme(description: str, imported: BundleImport) -> str: """The bundle's README: its description, where it came from, how to run and share it, what it holds, then the import's report.""" diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index c1c6d1f55..9339ed962 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -295,7 +295,6 @@ def write(self, description: str, source: dict) -> BundleImport: pinned=tuple(self.pinned), floating=tuple(self.floating), dropped=tuple(self.dropped), needs=tuple(self.needs), undeclared=tuple(self.undeclared), duplicates=tuple(self.duplicates), downloaded=downloaded) - self.files[PROVENANCE] = provenance(source, imported) self.files["README.md"] = readme(description, imported) try: for folder, (name, toml) in self.tomls.items(): @@ -304,6 +303,7 @@ def write(self, description: str, source: dict) -> BundleImport: for path, text in self.files.items(): (staging / path).parent.mkdir(parents=True, exist_ok=True) (staging / path).write_text(text, encoding="utf-8") + (staging / PROVENANCE).write_text(provenance(source, imported, staging), encoding="utf-8") except OSError as e: raise BundleError([f"{show(self.dest.parent)}: can't write the import there: {os_reason(e)}"]) from None check_bundle(resolve_bundle(parse_bundle(staging)), installed=installed_bundles) diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 219c79d97..ef037eae8 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -31,6 +31,7 @@ from agent_env.store.local_state import holding_locks from agent_env.task.store import TASKS_COLLECTION +from ._fs import file_sha256 from .authoring import build_context_files from .parse import BundleKind from .plan import Plan, Write, artifact_writer, env_folder_writer, env_writer, folder_files, unpinned_store_refs @@ -127,7 +128,7 @@ def digest(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Digest | built = isinstance(write.source, BuiltImage) listing = build_context_files if built else folder_files for key, path in listing(self._plan.bundle_of(write.source.entry), write.source.entry).items(): - inputs["files"][key] = _file_sha256(path) + (_mode(path) if built else "") + inputs["files"][key] = file_sha256(path) + (_mode(path) if built else "") if (published := self._plan.published.get(write.id)) is not None: inputs["published"] = published.pinned if write.kind not in (BundleKind.TASK, BundleKind.EVAL): @@ -308,13 +309,5 @@ def _mode(path: Path) -> str: return "" if mode == _USUAL_MODE else f"+{mode:o}" -def _file_sha256(path: Path) -> str: - digest = hashlib.sha256() - with open(path, "rb") as file: - for chunk in iter(lambda: file.read(1 << 20), b""): - digest.update(chunk) - return "sha256:" + digest.hexdigest() - - def _now() -> str: return datetime.now(timezone.utc).isoformat() diff --git a/tst/unit/bundle/importer_entities_test.py b/tst/unit/bundle/importer_entities_test.py index 64bc0a58a..e786a80da 100644 --- a/tst/unit/bundle/importer_entities_test.py +++ b/tst/unit/bundle/importer_entities_test.py @@ -2,6 +2,7 @@ stored build context, downloading a context stored as one object once; and what a folder can't hold fails the import, unless --include-scope user leaves it in the store, pinned.""" +import hashlib import json import os import tomllib @@ -19,6 +20,7 @@ from agent_env.artifact.artifacts.skill import SkillArtifact from agent_env.artifact.store import get_artifact_store from agent_env.bundle import BundleError +from agent_env.bundle._import_report import store_fingerprint from agent_env.bundle.importer import import_task from agent_env.bundle.resolve import build_step from agent_env.cli import cli @@ -83,7 +85,7 @@ def test_with_the_user_scope_it_stays_in_the_store_pinned_and_the_report_and_rea ("artifact", "crm-image", 1, "envs/crm: image")] readme = (imported.root / "README.md").read_text() assert "## Still needs the store" in readme and "except what *Still needs the store* lists" in readme - assert json.loads((imported.root / "provenance.json").read_text())["store"][0]["id"] == "crm-image" + assert json.loads((imported.root / "provenance.json").read_text())["pinned"][0]["id"] == "crm-image" def test_the_cli_says_up_front_that_the_folder_still_needs_the_store(home): @@ -274,14 +276,43 @@ def test_provenance_names_each_written_entity_and_image_by_store_id_and_version_ text = (imported.root / "provenance.json").read_text() provenance = json.loads(text) + assert provenance["schema"] == 2 assert provenance["from"] == {"task": "t", "version": 1} + assert provenance["tasks"] == [{"file": "tasks/t.json", "id": "t", "version": 1}] and provenance["eval"] is None assert provenance["written"] == [{"folder": "envs/crm", "kind": "env", "type": "mcp_server", "id": "crm", "version": 1}] assert provenance["images"] == [{"folder": "envs/crm", "key": "image", "id": "crm-image", "version": 1, "dockerfile": "Dockerfile", "dockerfile_found": False}] + assert provenance["store"] == store_fingerprint() and provenance["store"].startswith("sha256:") assert "file://" not in text and str(home) not in text +def test_provenance_holds_each_file_the_import_wrote_as_written_so_an_edit_can_be_told(home): + """Every file but the README and provenance itself, by its path in the folder, as its content hash, +x when + executable.""" + MCPServerEnv.put(id="crm", docker_image_artifact=context_image(home, "crm-image", { + "Dockerfile": "FROM python:3.12-slim\nCOPY run.sh /\n", "run.sh": "#!/bin/sh\n"}), environment_name="crm") + _task([DEPLOY]) + + imported = import_task("t", home / "t") + + files = json.loads((imported.root / "provenance.json").read_text())["files"] + on_disk = {path.relative_to(imported.root).as_posix() for path in imported.root.rglob("*") if path.is_file()} + assert set(files) == on_disk - {"README.md", "provenance.json"} + assert {"tasks/t.json", "envs/crm/env.toml", "envs/crm/Dockerfile"} <= set(files) + for path, digest in files.items(): + executable = os.access(imported.root / path, os.X_OK) + expected = "sha256:" + hashlib.sha256((imported.root / path).read_bytes()).hexdigest() + assert digest == expected + ("+x" if executable else "") + + +def test_the_store_fingerprint_tells_one_configured_document_store_from_another(home, tmp_path, monkeypatch): + here = store_fingerprint() + monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "elsewhere")) + + assert store_fingerprint() != here and store_fingerprint() == store_fingerprint() + + def _saved_image(path, url, files): """A one-layer ``docker save`` tarball holding ``files``, stored at ``url``.""" path.write_bytes(saved_image([files], gz=True)) diff --git a/tst/unit/bundle/importer_test.py b/tst/unit/bundle/importer_test.py index e9ce9ba7c..3c7307857 100644 --- a/tst/unit/bundle/importer_test.py +++ b/tst/unit/bundle/importer_test.py @@ -288,6 +288,12 @@ def test_an_eval_imports_with_each_task_it_names_once_under_names_taken_from_the assert imported.eval == ImportedEval("evals/regression.toml", "nightly/regression", 1) plan = plan_bundle(resolve_bundle(parse_bundle(imported.root))) assert [entry.entry.name for entry in plan.evals] == ["regression"] + provenance = json.loads((imported.root / "provenance.json").read_text()) + assert provenance["eval"] == {"file": "evals/regression.toml", "id": "nightly/regression", "version": 1} + assert provenance["tasks"] == [{"file": "tasks/run.json", "id": "team/run", "version": 1}, + {"file": "tasks/run-2.json", "id": "other/run", "version": 1}, + {"file": "tasks/RUN-3.json", "id": "RUN", "version": 1}, + {"file": "tasks/triage.json", "id": "triage", "version": 1}] def test_an_eval_naming_a_task_the_store_doesnt_hold_is_refused_and_writes_nothing(store): From 4ff31614cf32c5142cb9e4cada7181f574d1f703 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 11:12:28 -0700 Subject: [PATCH 02/22] feat(bundle): agent-env bundle register writes a bundle into the configured stores The bundle is planned as a run plans it, then each @local id it writes takes a registry id: - an entry takes , the prefix the folder's name and a dash by default; - a task's output takes -; - a derived id keeps its suffix; - a shipped agent named by its alias takes the alias. Each task and eval names the version of what it references. Each image the bundle builds is built for linux/amd64, pushed under a tag of its sources and registered by digest with its build context. A ledger in the target store's bundle_registrations makes a rerun write only what changed. Problems are refused before anything is written: an id the store holds that the bundle didn't register (unless --adopt), an @local id left in a document, or, for a shared store, a credential written out or a URL only this machine reaches. A step that leaves its agent unset stays unset. Co-Authored-By: Claude Opus 5.5 --- .../artifact/artifacts/docker_image.py | 31 +- src/agent_env/bundle/_secrets.py | 24 ++ src/agent_env/bundle/ledger.py | 48 ++- src/agent_env/bundle/materialize.py | 94 +++- src/agent_env/bundle/plan.py | 16 +- src/agent_env/bundle/register.py | 406 ++++++++++++++++++ src/agent_env/cli/bundle.py | 60 ++- tst/unit/bundle/register_test.py | 396 +++++++++++++++++ 8 files changed, 1032 insertions(+), 43 deletions(-) create mode 100644 src/agent_env/bundle/register.py create mode 100644 tst/unit/bundle/register_test.py diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index a2bdc8377..663281ee8 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -22,9 +22,9 @@ from agent_env.store.ids import fs_safe, image_repository, is_local_id, local_image_repository from agent_env.store.image_store.local_registry_image_store import LocalRegistryImageStore from agent_env.store.image_store.oci_registry_credentials import is_loopback_host, names_registry, registry_host_from_ref -from agent_env.store.image_store.registry_api import pin_digest, pinned_from +from agent_env.store.image_store.registry_api import NoSuchImageError, pin_digest, pinned_from from agent_env.utils.docker.context import BuildContext -from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, normalize_platform +from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, build_image, normalize_platform logger = logging.getLogger(__name__) @@ -561,6 +561,33 @@ class GitHubBuildResult: docker_context_github_url: str | None +def push_context_image(id: str, context_path: str, dockerfile_path: str, + before: Callable[[], None] | None = None) -> tuple[str, str]: + """The ref and digest of the image of ``id``'s build context in ``id``'s image store, under the tag of its sources: + built for linux/amd64 with this machine's Docker and pushed, calling ``before`` first, unless the store serves the + tag already. Raises DockerBuildError when the build fails, RuntimeError when the push does, and ValueError when the + registry can't be read.""" + from agent_env.config import get_config + + context = _context_of(Path(context_path), dockerfile_path) + tag = "src-" + context.source_digest(DEFAULT_BUILD_PLATFORM).removeprefix("sha256:") + store = get_config().get_image_store_for(id) + # ECR, unlike the OCI spec, refuses two separators in a row, which a derived id (``__env_image``) holds. + repository = image_repository(id).replace("__", "-") + ref = store.image_ref(repository, tag) + store.ensure_repository(repository) + try: + return ref, pin_digest(ref, store.auth(ref)).rpartition("@")[2] + except NoSuchImageError: + pass + if before is not None: + before() + local = f"{local_image_repository(id)}:{tag}" + build_image(Path(context_path) / context.dockerfile, Path(context_path), local, platform=DEFAULT_BUILD_PLATFORM) + _push_local_image(local, ref, store) + return ref, pin_digest(ref, store.auth(ref)).rpartition("@")[2] + + def _push_local_image(local_tag: str, image_ref: str, image_store) -> None: auth = image_store.auth(image_ref) if auth is not None: diff --git a/src/agent_env/bundle/_secrets.py b/src/agent_env/bundle/_secrets.py index 22f1590c0..1a5a279cf 100644 --- a/src/agent_env/bundle/_secrets.py +++ b/src/agent_env/bundle/_secrets.py @@ -6,6 +6,7 @@ from typing import Any from agent_env.config.describe import is_secret_key +from agent_env.config.loader import SECRET_REF_PREFIX # The fields that map environment variable names to values, where a credential is most often set. _ENV_VAR_MAPS = ("env_vars", "default_env_vars") @@ -58,3 +59,26 @@ def _masked(text: str) -> tuple[str, list[str]]: if count: found.append(what) return text, found + + +def credentials_in(value: Any, path: str = "") -> list[tuple[str, str]]: + """Each credential ``value`` holds written out, by its path, and what it is: an env var whose name looks like a + secret's, set to anything but a ``secret:`` reference, and any value holding a credential ``scrub`` masks.""" + found = [] + items = value.items() if isinstance(value, dict) else enumerate(value) if isinstance(value, list) else () + for key, item in items: + at = f"{path}.{key}" if path and isinstance(key, str) else f"{path}[{key}]" if isinstance(key, int) else key + if isinstance(value, dict) and key in _ENV_VAR_MAPS and isinstance(item, dict): + for name, setting in item.items(): + if not isinstance(setting, str) or not setting or setting.startswith(SECRET_REF_PREFIX): + continue + if is_secret_key(name): + found.append((f"{at}.{name}", "its name looks like a secret's, and it's set")) + elif kinds := _masked(setting)[1]: + found.append((f"{at}.{name}", f"it holds {kinds[0]}")) + elif isinstance(item, str): + if kinds := _masked(item)[1]: + found.append((at, f"it holds {', '.join(kinds)}")) + else: + found += credentials_in(item, at) + return found diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 219c79d97..a6e3c9dba 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -27,7 +27,7 @@ from agent_env.env.store import ENVS_COLLECTION from agent_env.eval.store import EVALS_COLLECTION from agent_env.store import Filter, Sort -from agent_env.store.document_store import LocalSqliteDocumentStore +from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore from agent_env.store.local_state import holding_locks from agent_env.task.store import TASKS_COLLECTION @@ -79,7 +79,11 @@ def next_version(self) -> int: class Ledger: """One plan's rows in the ``@local`` namespace's store.""" - def __init__(self, store: LocalSqliteDocumentStore, plan: Plan): + collection = LEDGER_COLLECTION + # Whether a task or eval is written naming the versions of what it references, so is hashed with them. + pins_references = False + + def __init__(self, store: DocumentStore, plan: Plan): self._store = store self._plan = plan @@ -130,10 +134,10 @@ def digest(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Digest | inputs["files"][key] = _file_sha256(path) + (_mode(path) if built else "") if (published := self._plan.published.get(write.id)) is not None: inputs["published"] = published.pinned - if write.kind not in (BundleKind.TASK, BundleKind.EVAL): + if self.pins_references or write.kind not in (BundleKind.TASK, BundleKind.EVAL): # A document records the versions of what it references (an env its images, a universe its - # environments), so one written anew means it must be written again. A task or eval names its - # references without a version. + # environments), so one written anew means it must be written again. A task or eval a run writes names + # its references without a version. for store, id in write.needs: inputs["needs"][f"{store} {id}"] = str(needs[store, id]) for (kind, id), version in unpinned_store_refs(self._plan, write).items(): @@ -146,36 +150,44 @@ def record(self, check: Check, write: Callable[[], int]) -> int: A pending row marks the attempt until it's done, with its version once ``write`` returns. One an interrupted run of this bundle left is kept by ``check`` when its version can be, and dropped here.""" key = {"store": check.write.kind.store, "id": check.write.id, "bundle": self._bundle(check.write)} - self._store.ensure_index(LEDGER_COLLECTION, ["store", "id", "status"]) - while self._store.delete(LEDGER_COLLECTION, Filter.of(**key, status="pending")): + self._store.ensure_index(self.collection, ["store", "id", "status"]) + while self._store.delete(self.collection, Filter.of(**key, status="pending")): pass pending = {**key, "status": "pending", "scheme": SCHEME, "digest": check.digest.value if check.digest else None, - "inputs": check.digest.inputs if check.digest else None, "at": _now()} - self._store.insert(LEDGER_COLLECTION, pending) + "inputs": check.digest.inputs if check.digest else None, "at": _now(), **self.row_fields()} + self._store.insert(self.collection, pending) version = write() # Stamped before the check below, which can take seconds, so a run interrupted during it leaves the version # this attempt wrote, and the next run keeps it rather than writing it again. - self._store.replace(LEDGER_COLLECTION, Filter.of(**key, status="pending"), {**pending, "version": version}) + self._store.replace(self.collection, Filter.of(**key, status="pending"), {**pending, "version": version}) done = {**pending, "status": "done", "version": version, "at": _now()} if check.digest is not None and self.digest(check.write, check.needs) != check.digest: # A file changed during the write, so what the version holds is unknown: the next run writes it again. done.update(digest=None, inputs=None) - self._store.replace(LEDGER_COLLECTION, Filter.of(**key, status="pending"), done, upsert=True) + self._store.replace(self.collection, Filter.of(**key, status="pending"), done, upsert=True) return version def adopt(self, check: Check) -> None: """Record the version an interrupted run wrote, which ``check`` keeps, as that run would have.""" key = {"store": check.write.kind.store, "id": check.write.id, "bundle": self._bundle(check.write)} done = {**key, "status": "done", "scheme": SCHEME, "digest": check.digest.value, "inputs": check.digest.inputs, - "version": check.version, "at": _now()} - self._store.replace(LEDGER_COLLECTION, Filter.of(**key, status="pending"), done, upsert=True) + "version": check.version, "at": _now(), **self.row_fields()} + self._store.replace(self.collection, Filter.of(**key, status="pending"), done, upsert=True) + + def row_fields(self) -> dict[str, Any]: + """What each row records beyond the write: nothing here.""" + return {} + + def latest_row(self, store: str, id: str) -> dict | None: + """The latest done row for (``store``, ``id``), whichever bundle wrote it.""" + return self._latest(store, id) def _orphan(self, write: Write, stored: int | None, digest: Digest) -> int | None: """The version an interrupted run of this bundle wrote and didn't record, when it's still the store's latest and was made from what ``write`` is now made from.""" - pending = self._find(LEDGER_COLLECTION, Filter.of(store=write.kind.store, id=write.id, - bundle=self._bundle(write), status="pending")) + pending = self._find(self.collection, Filter.of(store=write.kind.store, id=write.id, + bundle=self._bundle(write), status="pending")) if pending is None or pending.get("version") is None or pending["version"] != stored: return None return stored if (pending["scheme"], pending["digest"]) == (SCHEME, digest.value) else None @@ -185,15 +197,15 @@ def _bundle(self, write: Write) -> str: return self._plan.bundle_of(write.source.entry).id_root def _latest(self, store: str, id: str) -> dict | None: - return self._find(LEDGER_COLLECTION, Filter.of(store=store, id=id, status="done")) + return self._find(self.collection, Filter.of(store=store, id=id, status="done")) def _stored_version(self, write: Write) -> int | None: doc = self._find(_COLLECTIONS[write.kind.store], Filter.of(id=write.id)) return doc["version"] if doc else None def _find(self, collection: str, filter: Filter) -> dict | None: - """The highest version matching ``filter``, read without creating the store.""" - if not self._store.path.exists(): + """The highest version matching ``filter``, read without creating a local store.""" + if isinstance(self._store, LocalSqliteDocumentStore) and not self._store.path.exists(): return None return self._store.find_one(collection, filter, sort=Sort.by("version")) diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index feffc6310..6e9f0c1cc 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -24,9 +24,9 @@ from agent_env.a2a_agent import A2AAgent from agent_env.artifact import Artifact -from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact, push_context_image from agent_env.artifact.registry import canonical_type, get_artifact_registry -from agent_env.entity.refs import EntityRef, RefRole, ref_sites +from agent_env.entity.refs import EntityKind, EntityRef, RefRole, ref_sites from agent_env.env.registry import get_env_registry from agent_env.eval import Eval, EvalTask from agent_env.providers.sandbox_providers.host_images import build, build_staged @@ -42,7 +42,15 @@ from .ledger import Ledger, locked_ids, materializing from .parse import BundleError, BundleKind from .plan import Plan, Write, artifact_writer, env_writer, unpinned_store_refs -from .resolve import BuiltImage, BundleImage, ReferencedImage, build_step +from .resolve import BuiltImage, BundleImage, ReferencedImage, Reference, build_step + +@dataclass(frozen=True) +class Registering: + """How a registration writes a plan: through ``ledger``, which keeps its rows in the target store, each task and + eval naming the version of each entity it references, and each image the bundle builds built here and pushed.""" + + ledger: Ledger + @dataclass(frozen=True) class Materialized: @@ -79,6 +87,7 @@ def materialize( on_build: Callable[[Write], None] | None = None, on_write: Callable[[Materialized], None] | None = None, build_here: frozenset[str] = frozenset(), + registering: Registering | None = None, ) -> Materialization: """Write ``plan``'s entities, tasks and evals. The first write that fails stops it, and every earlier one stays: they're in the ledger, so the next run reuses them. ``on_wait`` is called when another run @@ -90,12 +99,14 @@ def materialize( ``dry_run`` checks and preflights what a run would, and writes nothing: no entity, ledger row or lock. Each write gets the version it would reuse or the store's next, which another run can take first. A step - reading what the run would write first isn't preflighted, since the store doesn't hold it yet.""" + reading what the run would write first isn't preflighted, since the store doesn't hold it yet. + + ``registering`` writes as a registration does (``Registering``).""" _refuse_unwritable(plan) if not namespace_routing_enabled(): raise RuntimeError("materializing a bundle needs namespace routing, which the agent-env CLI turns on; " "call it inside agent_env.store.routing.namespace_routing()") - ledger = Ledger.for_plan(plan) + ledger = registering.ledger if registering is not None else Ledger.for_plan(plan) entities = [write for write in plan.writes if write.kind not in (BundleKind.TASK, BundleKind.EVAL)] tasks = [write for write in plan.writes if write.kind is BundleKind.TASK] evals = [write for write in plan.writes if write.kind is BundleKind.EVAL] @@ -119,14 +130,18 @@ def through_ledger(write: Write, write_fn: Callable[[], int]) -> None: with nullcontext() if dry_run else materializing(locked_ids(plan), on_wait): for write in entities: here = write.id in build_here and not dry_run - through_ledger(write, lambda: _write_entity(plan, write, here, on_build)) + if registering is not None and isinstance(write.source, BuiltImage) and write.id not in plan.published: + through_ledger(write, lambda: _write_pushed_image(plan, write, on_build)) + else: + through_ledger(write, lambda: _write_entity(plan, write, here, on_build)) if here and done[_key(write)].reused: _build_reused(plan, write, done[_key(write)].version, on_build) unwritten = {_key(item.write) for item in done.values() if not item.reused} if dry_run else set() + versions = {key: item.version for key, item in done.items()} if registering is not None else None built, problems, unchecked = {}, [], [] for write in tasks: with _noted(plan, write, "preflighting"): - built[write.id] = _build(write) + built[write.id] = _build(write, plan, versions) found, skipped = _preflight(plan, write, built[write.id], unwritten) problems.extend(f"{_path(plan, write)}: {problem}" for problem in found) unchecked.extend((write, step) for step in skipped) @@ -135,7 +150,9 @@ def through_ledger(write: Write, write_fn: Callable[[], int]) -> None: for write in tasks: through_ledger(write, lambda: Task.put(id=write.id, steps=built[write.id].steps).version) for write in evals: - through_ledger(write, lambda: _WRITERS[write.kind](plan, write)) + if versions is not None: + versions.update({key: item.version for key, item in done.items()}) + through_ledger(write, lambda: _write_eval(plan, write, versions)) return Materialization(plan, tuple(done[_key(write)] for write in plan.writes), tuple(unchecked)) @@ -170,6 +187,23 @@ def _write_build_context(plan: Plan, write: Write, here: bool, on_build: Callabl raise BundleError([f"{_path(plan, write)}: {e}"]) from None +def _write_pushed_image(plan: Plan, write: Write, on_build: Callable[[Write], None] | None) -> int: + """Register the image an entry's Dockerfile describes by its digest in the configured image store, built here and + pushed unless the store holds it already (``push_context_image``), with its build context beside it.""" + image = write.source + with staged_build_context(plan.bundle_of(image.entry), image.entry, image.dockerfile) as staged: + try: + ref, digest = push_context_image(write.id, str(staged.path), staged.dockerfile, _before(on_build, write)) + return DockerImageArtifact.put_ref( + write.id, description=_image_description(plan, write), image_name=ref, image_digest=digest, + context_path=str(staged.path), dockerfile_path=staged.dockerfile, + ).version + except DockerBuildError as e: + raise BundleError([f"{_path(plan, write)}: {_tail(str(e))}"]) from None + except (ValueError, RuntimeError) as e: # the push, the registry's answer, or the object store + raise BundleError([f"{_path(plan, write)}: {e}"]) from None + + def _write_published_image(plan: Plan, write: Write) -> int: """Register the image published for the sources an entry's Dockerfile builds, by its digest, with its build context beside it, which a sandbox that can't pull the image builds it from; nothing is built here. Refused when the @@ -276,13 +310,28 @@ def _pinned(plan: Plan, write: Write, refs: tuple[EntityRef, ...]) -> Any: return config -def _write_eval(plan: Plan, write: Write) -> int: - """An unpinned task ref, the bundle's own or a store's, is written without a version: the eval runs - its latest.""" - tasks = [EvalTask(ref.id, ref.version) for ref in write.source.references] +def _write_eval(plan: Plan, write: Write, versions: dict[tuple[str, str], int] | None = None) -> int: + """An unpinned task ref, the bundle's own or a store's, is written without a version, so the eval runs its latest; + given the ``versions`` written, as a registration is, each names the version it reads.""" + tasks = [EvalTask(ref.id, ref.version if versions is None else _version(plan, ref, versions)) + for ref in write.source.references] return Eval.put(id=write.id, tasks=tasks).version +def _version(plan: Plan, ref: Reference, versions: dict[tuple[str, str], int]) -> int: + """The version ``ref`` reads: its own, the one written for the bundle's entity it names, or the store's latest the + plan read.""" + if ref.version is not None: + return ref.version + if ref.local is not None: + return versions[_KINDS[ref.kind], ref.id] + return plan.store_latest[ref.kind, ref.id] + + +# The store each kind a reference names is written to, by the key ``done`` holds it under. +_KINDS = {EntityKind.ENV: "env", EntityKind.AGENT: "agent", EntityKind.ARTIFACT: "artifact", EntityKind.TASK: "task"} + + # The writers this release has, by kind; any other kind is refused before anything is written. Tasks are # written separately, once every one of them is preflighted, and evals after them, since they name the tasks. _WRITERS: dict[BundleKind, Callable[[Plan, Write], int]] = { @@ -321,8 +370,25 @@ def _noted(plan: Plan, write: Write, doing: str) -> Iterator[None]: raise -def _build(write: Write) -> Task: - return Task(id=write.id, version=None, steps=[build_step(step) for step in write.source.config]) +def _build(write: Write, plan: Plan, versions: dict[tuple[str, str], int] | None) -> Task: + """The task ``write`` puts: as resolved, or given the ``versions`` written, with each reference that takes a version + naming the one it reads.""" + steps = write.source.config if versions is None else pinned_steps(plan, write, versions) + return Task(id=write.id, version=None, steps=[build_step(step) for step in steps]) + + +def pinned_steps(plan: Plan, write: Write, versions: dict[tuple[str, str], int]) -> list[dict]: + """A copy of the task ``write``'s steps with each reference it reads, the bundle's own entity or a store's, naming + the version it reads (``_version``), where its field takes a version.""" + steps = copy.deepcopy(write.source.config) + references = {(ref.kind, ref.id): ref for ref in write.source.references} + registry = get_task_step_registry() + for step in steps: + for site in ref_sites(registry[step["type"]].entity_refs or (), step): + ref = references.get((site.ref.kind, site.value)) + if site.ref.role is RefRole.INPUT and site.version_key is not None and site.version is None and ref: + site.rewrite(site.value, _version(plan, ref, versions)) + return steps def _preflight(plan: Plan, write: Write, task: Task, diff --git a/src/agent_env/bundle/plan.py b/src/agent_env/bundle/plan.py index 0b56d3908..cb50c8477 100644 --- a/src/agent_env/bundle/plan.py +++ b/src/agent_env/bundle/plan.py @@ -187,13 +187,14 @@ def path(self, entry: BundleEntry) -> str: def plan_bundle(resolved: ResolvedBundle, *, tasks: Sequence[str] = (), evals: Sequence[str] = (), - installed: Installed | None = None, agent: str | None = None) -> Plan: + installed: Installed | None = None, agent: str | None = None, default_agents: bool = True) -> Plan: """Plan a run of ``tasks`` and ``evals``, each a name or id in this bundle. With neither, the run is every eval or, in a bundle without evals, every task. ``installed`` lists the installed bundles whose entities a reference pulls in (``installed_bundles``); without it, an id under one is a store id. ``agent`` names - the agent the solver runs as, as a task's reference would, leaving the tasks as written (``Plan.agent``). Raises - BundleError listing every problem found.""" - return _Planner(resolved, installed).plan(tasks, evals, agent) + the agent the solver runs as, as a task's reference would, leaving the tasks as written (``Plan.agent``). Without + ``default_agents``, a step that leaves its agent unset pulls in no configured default, as for a task written to + run wherever another's default applies. Raises BundleError listing every problem found.""" + return _Planner(resolved, installed, default_agents).plan(tasks, evals, agent) def plan_entities(resolved: ResolvedBundle, ids: Sequence[str], *, installed: Installed | None = None) -> Plan: @@ -220,8 +221,9 @@ class _Pullable: class _Planner: - def __init__(self, resolved: ResolvedBundle, installed: Installed | None = None): + def __init__(self, resolved: ResolvedBundle, installed: Installed | None = None, default_agents: bool = True): self.resolved = resolved + self.default_agents = default_agents self.problems: list[str] = [] self.nodes: dict[_Key, ResolvedEntry | BundleImage] = {} self.rank: dict[_Key, int] = {} @@ -271,7 +273,7 @@ def _localized(self, entry: ResolvedEntry) -> ResolvedEntry: reference to the bundle's own entity points at that; for a task, with a reference for each step that deploys the configured default agent when this bundle or an installed one defines it.""" references = [ref if ref.local is not None else self._pulled(ref) for ref in entry.references] - if entry.entry.kind is BundleKind.TASK: + if entry.entry.kind is BundleKind.TASK and self.default_agents: references += self._default_agents(entry) if references == list(entry.references): return entry @@ -325,7 +327,7 @@ def _resolve_aliases(self) -> dict[str, AliasUse]: aliased[ref.kind, ref.id] = use.target_id ref = self._pulled(replace(ref, id=use.target_id)) references.append(ref) - if _leaves_an_agent_unset(entry): + if self.default_agents and _leaves_an_agent_unset(entry): if default is _UNREAD: default = self._alias_use(self.alias_table, EntityKind.AGENT, get_config().get_default_a2a_agent_id(), None, self.alias_uses, diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py new file mode 100644 index 000000000..cf8739af2 --- /dev/null +++ b/src/agent_env/bundle/register.py @@ -0,0 +1,406 @@ +"""Register a bundle folder into the configured stores under registry ids, so whatever runs a store's tasks runs what the +folder ran. + +A registration plans every task and eval in the folder, or those selected, as a run plans them, but without the +configured default agent a step that leaves its agent unset falls back to: the registered task leaves it unset, so +whatever runs it takes its own default. Each ``@local`` id the plan writes then takes a registry id (``registry_ids``), +and each task and eval is written naming the version of each entity it references, so a registered version never +changes meaning. Each image the bundle builds is built here for linux/amd64 and pushed to the configured image store +under a tag of its sources, with its build context beside it; one published for its sources is registered as published. + +Nothing is written when any problem is found before the first write: an id the rules can't give, two writes given one +id, an id the store holds that this bundle didn't register (unless adopted), an ``@local`` id left in what would be +written, a store reference the store doesn't hold, or, for a store that isn't this machine's, a credential written out +or a URL that only reaches this machine. A dry run checks the same and writes, builds and pushes nothing. Each write +goes through a ledger kept in the target store, so registering again writes only what changed. Two registrations +racing for one id can't both write a version of it: the loser fails, and running it again reuses what the winner wrote. +""" + +from __future__ import annotations + +import re +import subprocess +import unicodedata +from collections.abc import Callable, Iterator, Mapping, Sequence +from dataclasses import dataclass, replace +from importlib.metadata import version as package_version +from pathlib import Path +from typing import Any +from urllib.parse import urlsplit + +from agent_env.config import get_config +from agent_env.entity.refs import RefRole, ref_sites +from agent_env.plugins._bundles import AGENT_ALIASES, ENV_ALIASES +from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore +from agent_env.store.ids import LOCAL_PREFIX, is_local_id +from agent_env.store.image_store.oci_registry_credentials import is_loopback_host +from agent_env.store.routing import configured_store, namespace_routing +from agent_env.task_step.registry import get_task_step_registry + +from ._secrets import credentials_in +from .installed import installed_bundles +from .ledger import Ledger +from .materialize import Materialization, Materialized, Registering, materialize +from .parse import Bundle, BundleEntry, BundleError, BundleKind, parse_bundle +from .plan import AliasUse, Plan, Write, plan_bundle, plan_entities +from .run import _label +from .resolve import BundleImage, Reference, ResolvedBundle, ResolvedEntry, resolve_bundle + +REGISTRATIONS = "bundle_registrations" +# A registry id: what a hub route, an object key and an image repository all take. +_REGISTRY_ID = re.compile(r"[a-z0-9][a-z0-9._-]*") +_NOT_IN_ID = re.compile(r"[^a-z0-9._-]+") +_URL = re.compile(r"\b(?:file:///?[^\s\"']*|[a-z][a-z0-9+.-]*://[^\s/\"']+)", re.IGNORECASE) + + +@dataclass(frozen=True) +class Registration: + """What registering a bundle wrote, or in a dry run would write, under which registry ids.""" + + plan: Plan # under the registry ids + materialization: Materialization + ids: Mapping[str, str] # each ``@local`` id the plan writes or its tasks' outputs take, and its registry id + floating: tuple[str, ...] # each reference written without a version, so whatever runs it reads its latest + bundle: str # the bundle's identity, which its ledger rows name + shared: bool # the target is a store other than this machine's own + + +def register_bundle( + root: Path | str, + *, + tasks: Sequence[str] = (), + evals: Sequence[str] = (), + prefix: str | None = None, + adopt: Sequence[str] = (), + dry_run: bool = False, + on_progress: Callable[[str], None] | None = None, +) -> Registration: + """Register the bundle at ``root``: ``tasks`` and ``evals`` by name, or with neither every task and eval it has, + or every entity when it has none, with what they reference. ``prefix`` opens each entry's registry id, the folder's + name and a dash unless given. ``adopt`` names ids the store holds that this bundle may write new versions of though + it didn't register them. ``on_progress`` is given a line for each write, as it's checked or written, and before + each image is built. + + It turns namespace routing on while it runs. Raises BundleError listing every problem found before anything is + written; a write that fails stops it, and every earlier one stays, in the ledger, for the next run to reuse.""" + with namespace_routing(): + bundle = parse_bundle(Path(root)) + resolved = resolve_bundle(bundle) + plan = _plan(resolved, tasks, evals) + ids, problems = registry_ids(plan, _prefix(bundle, prefix)) + if problems: + raise BundleError(problems) + registered = translated(plan, ids) + target = configured_store(get_config().get_document_store()) + shared = shared_store(target) + identity, commit, dirty = bundle_identity(bundle.root) + ledger = RegistrationLedger(target, registered, identity, commit, dirty) + problems = [*_unowned(registered, ledger, frozenset(adopt)), *_unregistrable(registered, shared)] + if problems: + raise BundleError(problems) + say = on_progress or (lambda line: None) + materialization = materialize( + registered, dry_run=dry_run, registering=Registering(ledger), + on_wait=lambda: say("waiting for another agent-env run to finish writing this bundle's ids"), + on_build=lambda write: say(f" {_label(registered, write)}: building with docker and pushing"), + on_write=lambda done: say(f" {registered_line(registered, done)}")) + return Registration(registered, materialization, ids, tuple(_floating(registered)), identity, shared) + + +def shared_store(store: DocumentStore) -> bool: + """Whether ``store`` is other than this machine's own, so others read what's written to it.""" + return not isinstance(store, LocalSqliteDocumentStore) + + +def registered_line(plan: Plan, done: Materialized) -> str: + """A write, as a registration names it: where it's written from, the registry id and version, and why.""" + why = "unchanged" if done.reused else "; ".join(done.reasons) + return f"{_label(plan, done.write)} -> {done.write.kind.store} {done.write.id} v{done.version} ({why})" + + +def _plan(resolved: ResolvedBundle, tasks: Sequence[str], evals: Sequence[str]) -> Plan: + """The plan of ``tasks`` and ``evals``, or of every task and eval, or of every entity of a bundle with none.""" + entries = resolved.bundle.entries + if not tasks and not evals: + tasks = [entry.name for entry in entries if entry.kind is BundleKind.TASK] + evals = [entry.name for entry in entries if entry.kind is BundleKind.EVAL] + if not tasks and not evals: + return plan_entities(resolved, [entry.id for entry in entries], installed=installed_bundles) + return plan_bundle(resolved, tasks=tasks, evals=evals, installed=installed_bundles, default_agents=False) + + +def _prefix(bundle: Bundle, prefix: str | None) -> str: + """``prefix``, or the folder's name as an id and a dash.""" + if prefix is not None: + if prefix and not _REGISTRY_ID.fullmatch(prefix): + raise BundleError([f"--prefix {prefix!r}: an id opens with a lowercase letter or digit and holds only " + "lowercase letters, digits and . _ -"]) + return prefix + name = _slug(bundle.name) + if not name: + raise BundleError([f"{bundle.name!r}: the folder's name gives no id; pass --prefix"]) + return f"{name}-" + + +def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: + """The registry id each ``@local`` id the plan writes takes, and each problem giving them. + + The bundle's own entry is ````, its name lowercased with each run of what an id can't hold made + ``-``. An installed bundle's agent or env pulled in by a reference takes the first alias its bundle gives it, + which the reference named it by. An id derived from another (``__env_image``) keeps its suffix after the + other's registry id, and a task's output is ``-``.""" + ids: dict[str, str] = {} + problems: list[str] = [] + for write in plan.writes: + entry = write.source.entry + if entry.id in ids or (isinstance(write.source, BundleImage) and write.id != entry.id): + continue + if entry in plan.origins: + alias = _alias(plan.origins[entry], entry) + if alias is None: + problems.append(f"{plan.path(entry)}: comes with an installed bundle and has no alias to register it " + "under; name an entity the store holds instead") + continue + ids[entry.id] = alias + elif name := _slug(entry.name): + ids[entry.id] = f"{prefix}{name}" + else: + problems.append(f"{plan.path(entry)}: its name gives no id; rename it") + for write in plan.writes: + if write.id not in ids and (derived := _derived(write.id, ids)) is not None: + ids[write.id] = derived + source = write.source + if isinstance(source, ResolvedEntry) and source.entry.id in ids: + for output in source.outputs: + name = _slug(output.id.removeprefix(f"{source.entry.id}/").rsplit("/", 1)[-1]) + ids[output.id] = f"{ids[source.entry.id]}-{name}" + by_target: dict[str, list[str]] = {} + for local, registry_id in ids.items(): + by_target.setdefault(registry_id, []).append(local) + if not _REGISTRY_ID.fullmatch(registry_id.replace("__", "_")): + problems.append(f"{registry_id!r}: isn't an id a registry takes; rename what it's made from") + for registry_id, locals_ in by_target.items(): + if len(locals_) > 1: + problems.append(f"{', '.join(sorted(locals_))} would all be registered as {registry_id!r}; rename one") + return ids, problems + + +def _alias(bundle: Bundle, entry: BundleEntry) -> str | None: + table = {BundleKind.AGENT: AGENT_ALIASES, BundleKind.ENV: ENV_ALIASES}.get(entry.kind) + aliases = bundle.aliases.get(table, {}).get(entry.name, ()) if table else () + return aliases[0] if aliases else None + + +def _derived(entity_id: str, ids: Mapping[str, str]) -> str | None: + """The registry id of ``entity_id`` when it's derived from an id in ``ids``: that id's, with the same suffix.""" + base = max((known for known in ids if entity_id.startswith(f"{known}__")), key=len, default=None) + return None if base is None else ids[base] + entity_id[len(base):] + + +def _slug(name: str) -> str: + """``name`` lowercased, in ASCII, with each run of what an id can't hold made ``-``, and no ``-``, ``.`` or ``_`` + at its ends.""" + ascii_name = unicodedata.normalize("NFKD", name).encode("ascii", "ignore").decode().lower() + return _NOT_IN_ID.sub("-", ascii_name.replace("__", "-")).strip("-._") + + +def translated(plan: Plan, ids: Mapping[str, str]) -> Plan: + """``plan`` writing each entity under its registry id, and naming it so wherever the plan's documents do.""" + + def entity(entry: BundleEntry) -> BundleEntry: + if entry not in entries: + entries[entry] = replace(entry, id=ids.get(entry.id, entry.id)) + return entries[entry] + + def image(source: BundleImage) -> BundleImage: + return replace(source, id=ids.get(source.id, source.id), entry=entity(source.entry)) + + def reference(ref: Reference) -> Reference: + local = ref.local + if isinstance(local, BundleImage): + local = image(local) + elif isinstance(local, BundleEntry): + local = entity(local) + return replace(ref, id=ids.get(ref.id, ref.id), local=local) + + def resolved(source: ResolvedEntry) -> ResolvedEntry: + return ResolvedEntry(entity(source.entry), _renamed(source.config, ids), + tuple(reference(ref) for ref in source.references), + tuple(replace(output, id=ids.get(output.id, output.id)) for output in source.outputs)) + + def source_of(source: ResolvedEntry | BundleImage) -> ResolvedEntry | BundleImage: + return image(source) if isinstance(source, BundleImage) else resolved(source) + + entries: dict[BundleEntry, BundleEntry] = {} + writes = tuple(Write(write.kind, ids.get(write.id, write.id), source_of(write.source), + tuple((store, ids.get(id, id)) for store, id in write.needs)) for write in plan.writes) + sources = {write.source.entry: write.source for write in writes if isinstance(write.source, ResolvedEntry)} + own = plan.bundle.bundle + bundle = replace(own, entries=tuple(entity(entry) for entry in own.entries)) + resolved_bundle = ResolvedBundle(bundle, tuple(sources.get(entity(entry.entry)) or resolved(entry) + for entry in plan.bundle.entries), + tuple(image(source) for source in plan.bundle.images)) + return replace( + plan, bundle=resolved_bundle, writes=writes, + tasks=tuple(sources[entity(task.entry)] for task in plan.tasks), + evals=tuple(sources[entity(item.entry)] for item in plan.evals), + store_refs=tuple(reference(ref) for ref in plan.store_refs), + origins={entity(entry): origin for entry, origin in plan.origins.items()}, + published={ids.get(id, id): publication for id, publication in plan.published.items()}, + aliases={alias: _aliased(use, ids) for alias, use in plan.aliases.items()}, + env_aliases={alias: _aliased(use, ids) for alias, use in plan.env_aliases.items()}, + ) + + +def _aliased(use: AliasUse, ids: Mapping[str, str]) -> AliasUse: + return replace(use, target_id=ids.get(use.target_id, use.target_id)) + + +def _renamed(value: Any, ids: Mapping[str, str]) -> Any: + """A copy of ``value`` with each string that's an ``@local`` id in ``ids``, or derived from one, its registry id.""" + if isinstance(value, str): + return ids.get(value) or _derived(value, ids) or value if is_local_id(value) else value + if isinstance(value, dict): + return {key: _renamed(item, ids) for key, item in value.items()} + if isinstance(value, list): + return [_renamed(item, ids) for item in value] + return value + + +class RegistrationLedger(Ledger): + """A registration's rows, in the target store: each names the bundle that wrote it, by its identity, and the commit + and state of the folder it was written from. A task or eval is hashed with the versions it names.""" + + collection = REGISTRATIONS + pins_references = True + + def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None): + super().__init__(store, plan) + self.identity, self.commit, self.dirty = identity, commit, dirty + + def bundle_key(self, write: Write) -> str: + return self._bundle(write) + + def stored_version(self, write: Write) -> int | None: + return self._stored_version(write) + + def _bundle(self, write: Write) -> str: + """The bundle's identity, or for an entity an installed bundle comes with, that bundle's, which every bundle + registering it shares.""" + origin = self._plan.origins.get(write.source.entry) + return f"installed:{origin.id_root.removeprefix(LOCAL_PREFIX)}" if origin is not None else self.identity + + def row_fields(self) -> dict[str, Any]: + return {"commit": self.commit, "dirty": self.dirty, "agent_env": package_version("agentenv-framework")} + + +def bundle_identity(root: Path) -> tuple[str, str | None, bool | None]: + """Who the bundle at ``root`` is, which its ledger rows name: its git remote, without credentials, and its path in + the repo, or else ``folder:`` and its path on this machine; with the commit checked out and whether the folder + holds uncommitted changes, both None outside git.""" + remote = _git(root, "remote", "get-url", "origin") + path_in_repo = _git(root, "rev-parse", "--show-prefix") + if remote is None or path_in_repo is None: + return f"folder:{root}", None, None + status = _git(root, "status", "--porcelain", "--", ".") + return (f"{_remote_name(remote)}:{path_in_repo.rstrip('/')}", _git(root, "rev-parse", "HEAD"), + None if status is None else bool(status)) + + +def _git(root: Path, *args: str) -> str | None: + try: + result = subprocess.run(["git", "-C", str(root), *args], capture_output=True, text=True, timeout=10) + except (OSError, subprocess.TimeoutExpired): + return None + return result.stdout.strip() if result.returncode == 0 else None + + +def _remote_name(remote: str) -> str: + """A git remote as ``host/path``, without its scheme, credentials, port or ``.git``.""" + if "://" in remote: + parts = urlsplit(remote) + host, path = parts.hostname or "", parts.path + else: # scp-like, user@host:path + host, _, path = remote.rpartition("@")[2].partition(":") + return f"{host}/{path.strip('/').removesuffix('.git')}" + + +def _unowned(plan: Plan, ledger: RegistrationLedger, adopt: frozenset[str]) -> Iterator[str]: + """A problem for each write whose id the store holds already, unless this bundle registered it or ``adopt`` names + it.""" + for write in plan.writes: + stored = ledger.stored_version(write) + if stored is None or write.id in adopt: + continue + row = ledger.latest_row(write.kind.store, write.id) + if row is not None and row.get("bundle") == ledger.bundle_key(write): + continue + by = f"registered from {row['bundle']}" if row is not None else "not registered from a bundle" + yield (f"{plan.path(write.source.entry)}: {write.id!r} is in the store already, at v{stored}, {by}; pass " + f"--adopt {write.id} to write a new version of it, or --prefix to register this bundle under other ids") + + +def _unregistrable(plan: Plan, shared: bool) -> Iterator[str]: + """A problem for each ``@local`` id left in what the plan writes and, for a ``shared`` store, each credential + written out and each URL that reaches only this machine.""" + for write in plan.writes: + if not isinstance(write.source, ResolvedEntry): + continue + where = plan.path(write.source.entry) + for path, text in _strings(write.source.config): + if is_local_id(text): + yield f"{where}: {path or 'its value'} names {text!r}, which only this machine's stores hold" + elif shared and (url := _local_url(text)) is not None: + yield f"{where}: {path or 'its value'} names {url}, which only this machine reaches" + if shared: + for path, why in credentials_in(write.source.config): + yield (f"{where}: {path}: {why}, and a shared store shows it to anyone who reads it; leave it out, for " + "whatever runs the task to give it") + + +def _strings(value: Any, path: str = "") -> Iterator[tuple[str, str]]: + if isinstance(value, str): + yield path, value + elif isinstance(value, dict): + for key, item in value.items(): + yield from _strings(item, f"{path}.{key}" if path else str(key)) + elif isinstance(value, list): + for index, item in enumerate(value): + yield from _strings(item, f"{path}[{index}]") + + +def _local_url(text: str) -> str | None: + """The first URL in ``text`` that reaches only this machine: a file, or a loopback host's.""" + for match in _URL.finditer(text): + url = match.group(0) + if url.lower().startswith("file:") or is_loopback_host(urlsplit(url).hostname or ""): + return url + return None + + +def _floating(plan: Plan) -> Iterator[str]: + """Each entity a registered task names in a field that takes no version, so whatever runs it reads its latest.""" + registry = get_task_step_registry() + for write in plan.writes: + if write.kind is not BundleKind.TASK: + continue + for step in write.source.config: + for site in ref_sites(registry[step["type"]].entity_refs or (), step): + if site.ref.role is RefRole.INPUT and site.version_key is None and isinstance(site.value, str): + yield (f"{plan.path(write.source.entry)}: step {step.get('id')!r}: {site.path}: " + f"{site.ref.kind.value} {site.value}") + + +def describe_target() -> list[str]: + """The configured stores a registration writes to, as lines: their kinds, and where each is.""" + config = get_config() + documents = configured_store(config.get_document_store()) + objects = configured_store(config.get_object_store()) + images = configured_store(config.get_image_store()) + where = (str(documents.path) if isinstance(documents, LocalSqliteDocumentStore) + else getattr(getattr(documents, "database", None), "name", None)) + return [f"documents: {type(documents).__name__}" + (f" ({where})" if where else ""), + f"objects: {type(objects).__name__} ({objects.object_url('')})", + f"images: {type(images).__name__}" + (f" ({images.registry_host})" if hasattr(images, "registry_host") + else "")] + diff --git a/src/agent_env/cli/bundle.py b/src/agent_env/cli/bundle.py index c0253bdb2..ec4998c26 100644 --- a/src/agent_env/cli/bundle.py +++ b/src/agent_env/cli/bundle.py @@ -1,4 +1,4 @@ -"""``agent-env bundle``: start a bundle folder from what a store holds.""" +"""``agent-env bundle``: start a bundle folder from what a store holds, and register one into the stores.""" from __future__ import annotations @@ -7,13 +7,15 @@ import click from agent_env.bundle.importer import BundleImport, import_eval, import_task +from agent_env.bundle.parse import BundleKind +from agent_env.bundle.register import describe_target, register_bundle from agent_env.cli.utils import interruptible from agent_env.entity.scopes import FieldScope @click.group() def bundle(): - """Create bundle folders, which ``agent-env run`` runs.""" + """Create bundle folders, which ``agent-env run`` runs, and register them into the configured stores.""" @bundle.command("init") @@ -59,3 +61,57 @@ def _report(imported: BundleImport, folder: Path) -> None: for line in lines: click.echo(f" {line}") click.echo(f"Run it with: agent-env run {folder}") + + +@bundle.command("register") +@click.argument("folder", type=click.Path(exists=True, file_okay=False, path_type=Path)) +@click.option("--task", "tasks", multiple=True, + help="Register this task, by name, with what it uses. Repeatable. With no --task or --eval, every task " + "and eval in the bundle.") +@click.option("--eval", "evals", multiple=True, help="Register this eval, by name, and the tasks it names. Repeatable.") +@click.option("--prefix", default=None, + help="What each entry's registry id opens with; by default the folder's name and a dash.") +@click.option("--adopt", multiple=True, + help="An id the store holds that this bundle didn't register, to write a new version of. Repeatable.") +@click.option("--dry-run", is_flag=True, help="Check everything and say what would be written; write nothing.") +@click.option("--yes", is_flag=True, help="Write to a store other than this machine's own without asking.") +def register(folder: Path, tasks: tuple[str, ...], evals: tuple[str, ...], prefix: str | None, + adopt: tuple[str, ...], dry_run: bool, yes: bool): + """Write the bundle at FOLDER into the configured stores under registry ids, so whatever runs the stores' tasks + runs what the folder runs. + + Each env, agent, artifact, skill, task and eval takes the id PREFIX plus its name, lowercased; a task's outputs + take the task's id and their names. A shipped agent a task names by its alias is registered under the alias when + the store lacks one. Each task and eval names the version of everything it references, so a registered version + never changes meaning; a step that leaves its agent unset is left so, for whatever runs it to give its own. Each + image the bundle builds is built here for linux/amd64 with Docker and pushed to the configured image store, with + its build context beside it. + + It checks everything first and says what it would write: an id the store holds that this bundle didn't register, + an @local id left in what it would write, or, for a store other than this machine's, a credential written out or a + URL only this machine reaches, refuses the whole registration. A store other than this machine's is written only + after you confirm, or with --yes. Registering again writes only what changed since.""" + options = {"tasks": tasks, "evals": evals, "prefix": prefix, "adopt": adopt} + click.echo(f"Registering {folder} into:") + for line in describe_target(): + click.echo(f" {line}") + click.echo("Checking:") + with interruptible(): + planned = register_bundle(folder, dry_run=True, on_progress=click.echo, **options) + for line in planned.floating: + click.echo(f" floating, since its field takes no version: {line}") + if dry_run: + click.echo("Nothing was written: --dry-run.") + return + if all(done.reused for done in planned.materialization.writes): + click.echo("Nothing to write: every version is registered already.") + return + if planned.shared and not yes: + click.confirm("Write these to the stores above?", abort=True) + click.echo("Writing:") + with interruptible(): + registered = register_bundle(folder, on_progress=click.echo, **options) + tasks_written = [done for done in registered.materialization.writes if done.write.kind is BundleKind.TASK] + if tasks_written: + first = tasks_written[0] + click.echo(f"Run a registered task with: agent-env task run --id {first.write.id} --version {first.version}") diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py new file mode 100644 index 000000000..e68aa109e --- /dev/null +++ b/tst/unit/bundle/register_test.py @@ -0,0 +1,396 @@ +"""Registering a bundle: each entry under a registry id, each task and eval naming the versions it reads, a ledger in +the target store so registering again writes only what changed, and every problem refused before anything is +written.""" + +import json +import subprocess +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from agent_env.a2a_agent import A2AAgent +from agent_env.artifact import Artifact, FileArtifact +from agent_env.artifact.artifacts import docker_image as docker_image_module +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact, push_context_image +from agent_env.bundle import BundleError +from agent_env.bundle import materialize as materialize_module +from agent_env.bundle import register as register_module +from agent_env.bundle.register import REGISTRATIONS, _remote_name, register_bundle +from agent_env.cli import cli +from agent_env.config import get_config +from agent_env.config.runtime import Config +from agent_env.entity.refs import EntityRef, RefRole +from agent_env.env.env import Env +from agent_env.eval import Eval +from agent_env.store import Filter +from agent_env.task import Task +from agent_env.task_step.task_step import TaskStep +from tst.unit.bundle._support import DIGEST, layout + +BOX = {"id": "box", "type": "deploy_sandbox", "sandbox_name": "box", "sandbox_mode": "vm"} + + +def _load(step_id, artifact): + return {"id": step_id, "type": "load_artifact", "sandbox_name": "box", "artifact_id": artifact} + + +TRIAGE = { + "artifacts/greeting/hello.txt": "hello\n", + "artifacts/docs/a.md": "a\n", + "tasks/t.json": json.dumps([BOX, _load("greet", "greeting"), _load("docs", "docs")]), +} + + +class _Writes(TaskStep): + """A step writing the artifact ``artifact_id``.""" + + type = "writes_register_test" + entity_refs = (EntityRef.artifact("artifact_id", role=RefRole.OUTPUT),) + + def __init__(self, artifact_id=None, **base): + super().__init__(**base) + self.artifact_id = artifact_id + + @classmethod + def from_dict(cls, data): + return cls(**cls._base_from_dict(data), artifact_id=data.get("artifact_id")) + + def to_dict(self): + return {**super().to_dict(), "artifact_id": self.artifact_id} + + async def execute(self, context): + return context + + +@pytest.fixture(autouse=True) +def steps(monkeypatch): + registry = {**Config().task_step_registry(), _Writes.type: _Writes} + monkeypatch.setattr(Config, "task_step_registry", lambda self: registry) + + +_GIT = register_module._git + + +@pytest.fixture(autouse=True) +def outside_git(monkeypatch): + """Each folder is outside git, but the test that says otherwise: none reads a repository it didn't make.""" + monkeypatch.setattr(register_module, "_git", lambda root, *args: None) + + +@pytest.fixture +def home(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + return tmp_path + + +def _steps(task_id, version=None): + return Task.get(task_id, version).to_dict()["steps"] + + +def _rows(): + return get_config().get_document_store().query(REGISTRATIONS, Filter.of(status="done")) + + +def test_each_entry_takes_its_prefix_and_name_and_each_task_names_the_versions_it_reads(home): + registered = register_bundle(layout(home / "triage", TRIAGE)) + + assert {done.write.id: done.version for done in registered.materialization.writes} == { + "triage-docs": 1, "triage-greeting": 1, "triage-t": 1} + steps = _steps("triage-t") + assert [step["artifacts"] for step in steps[1:]] == [[{"id": "triage-greeting", "version": 1}], + [{"id": "triage-docs", "version": 1}]] + assert Artifact.get("triage-docs").version == 1 + + +def test_registering_again_writes_nothing_and_an_edit_rewrites_only_what_it_reaches(home): + root = layout(home / "triage", TRIAGE) + register_bundle(root) + + again = register_bundle(root) + (root / "artifacts/docs/a.md").write_text("changed\n") + edited = register_bundle(root) + + assert all(done.reused for done in again.materialization.writes) + assert {done.write.id: (done.version, done.reused) for done in edited.materialization.writes} == { + "triage-docs": (2, False), "triage-greeting": (1, True), "triage-t": (2, False)} + assert _steps("triage-t")[2]["artifacts"] == [{"id": "triage-docs", "version": 2}] + assert _steps("triage-t", 1)[2]["artifacts"] == [{"id": "triage-docs", "version": 1}] + + +def test_a_dry_run_says_what_it_would_write_and_writes_nothing(home): + planned = register_bundle(layout(home / "triage", TRIAGE), dry_run=True) + + assert {done.write.id: done.version for done in planned.materialization.writes} == { + "triage-docs": 1, "triage-greeting": 1, "triage-t": 1} + assert get_config().get_document_store().count("tasks", Filter()) == 0 and _rows() == [] + + +def test_the_ledger_rows_are_in_the_target_store_and_name_the_bundle_by_its_remote_without_credentials( + local_stores, tmp_path, monkeypatch): + monkeypatch.setattr(register_module, "_git", _GIT) + repo = tmp_path / "repo" + layout(repo / "bundles" / "triage", TRIAGE) + for args in (["init", "-q"], ["remote", "add", "origin", "https://user:token123@github.com/team/repo.git"], + ["add", "."], ["-c", "user.email=a@b.c", "-c", "user.name=a", "commit", "-q", "-m", "x"]): + subprocess.run(["git", "-C", str(repo), *args], check=True) + + registered = register_bundle(repo / "bundles" / "triage") + + assert registered.bundle == "github.com/team/repo:bundles/triage" + rows = _rows() + assert {row["id"] for row in rows} == {"triage-docs", "triage-greeting", "triage-t"} + assert {row["bundle"] for row in rows} == {"github.com/team/repo:bundles/triage"} + assert all(row["commit"] and row["dirty"] is False for row in rows) + assert "token123" not in json.dumps(rows, default=str) + + +@pytest.mark.parametrize(("remote", "name"), [ + ("git@github.com:team/repo.git", "github.com/team/repo"), + ("https://x-access-token:ghs_abc@github.com/team/repo", "github.com/team/repo"), + ("ssh://git@gitlab.example.com:2222/group/sub/repo.git", "gitlab.example.com/group/sub/repo"), +]) +def test_a_remote_is_named_by_its_host_and_path_alone(remote, name): + assert _remote_name(remote) == name + + +def test_an_id_the_store_holds_that_the_bundle_didnt_register_is_refused_unless_adopted(home): + (home / "seed.txt").write_text("seed") + FileArtifact.put("triage-docs", description="someone else's", file_path=str(home / "seed.txt")) + root = layout(home / "triage", TRIAGE) + + with pytest.raises(BundleError) as refused: + register_bundle(root) + adopted = register_bundle(root, adopt=["triage-docs"]) + + assert refused.value.problems == ( + "artifacts/docs: 'triage-docs' is in the store already, at v1, not registered from a bundle; pass --adopt " + "triage-docs to write a new version of it, or --prefix to register this bundle under other ids",) + assert {done.write.id: done.version for done in adopted.materialization.writes}["triage-docs"] == 2 + + +def test_another_folder_of_the_same_name_is_refused_the_ids_this_one_registered(home): + register_bundle(layout(home / "a" / "triage", TRIAGE)) + + with pytest.raises(BundleError) as refused: + register_bundle(layout(home / "b" / "triage", TRIAGE)) + + assert len(refused.value.problems) == 3 + assert all(f"registered from folder:{home / 'a' / 'triage'}" in problem for problem in refused.value.problems) + + +def test_a_prefix_registers_under_other_ids(home): + registered = register_bundle(layout(home / "triage", TRIAGE), prefix="team.") + + assert {done.write.id for done in registered.materialization.writes} == {"team.docs", "team.greeting", "team.t"} + + +def test_a_name_an_id_cant_hold_is_made_one_and_names_that_collide_are_refused(home): + files = {"artifacts/My Docs/a.md": "a\n", "artifacts/my-docs/b.md": "b\n", + "tasks/T.json": json.dumps([BOX, _load("one", "My Docs"), _load("two", "my-docs")])} + + with pytest.raises(BundleError) as refused: + register_bundle(layout(home / "Big Triage", files)) + + assert refused.value.problems == ("@local/~/Big Triage/My Docs, @local/~/Big Triage/my-docs would all be " + "registered as 'big-triage-my-docs'; rename one",) + + +def test_a_bad_prefix_is_refused(home): + with pytest.raises(BundleError) as refused: + register_bundle(layout(home / "triage", TRIAGE), prefix="Team/") + + assert "--prefix 'Team/'" in refused.value.problems[0] + + +def test_a_task_output_is_named_after_its_task_and_isnt_pinned(home): + files = {**TRIAGE, "tasks/t.json": json.dumps([BOX, {"id": "make", "type": _Writes.type, "artifact_id": "report"}, + {**_load("read", "report"), "depends_on": [{"task_step_id": "make"}]}])} + + registered = register_bundle(layout(home / "triage", files)) + + assert registered.ids["@local/~/triage/t/report"] == "triage-t-report" + steps = _steps("triage-t") + assert steps[1]["artifact_id"] == "triage-t-report" + assert steps[2]["artifacts"] == [{"id": "triage-t-report", "version": None}] + + +def test_an_eval_names_the_version_of_each_task(home): + files = {**TRIAGE, "evals/nightly.toml": 'tasks = ["t"]\n'} + root = layout(home / "triage", files) + register_bundle(root) + (root / "artifacts/docs/a.md").write_text("changed\n") + + register_bundle(root) + + assert [(task.task_id, task.task_version) for task in Eval.get("triage-nightly").tasks] == [("triage-t", 2)] + assert [(task.task_id, task.task_version) for task in Eval.get("triage-nightly", 1).tasks] == [("triage-t", 1)] + + +def test_a_store_reference_is_pinned_to_the_version_the_plan_read(home): + (home / "seed.txt").write_text("seed") + FileArtifact.put("shared-seed", description="seed", file_path=str(home / "seed.txt")) + FileArtifact.put("shared-seed", description="seed", file_path=str(home / "seed.txt")) + files = {"tasks/t.json": json.dumps([BOX, _load("seed", "shared-seed")])} + + register_bundle(layout(home / "triage", files)) + + assert _steps("triage-t")[1]["artifacts"] == [{"id": "shared-seed", "version": 2}] + + +def test_a_step_that_leaves_its_agent_unset_is_registered_so_and_writes_no_default_agent(home): + files = {"tasks/t.json": json.dumps([BOX, {"id": "agent", "type": "deploy_agent", "env_ids": []}])} + + registered = register_bundle(layout(home / "triage", files)) + + assert [done.write.id for done in registered.materialization.writes] == ["triage-t"] + assert _steps("triage-t")[1].get("a2a_agent_id") is None + + +def test_a_shipped_agent_named_by_its_alias_is_registered_under_the_alias(home, agents_published): + files = {"tasks/t.json": json.dumps([BOX, {"id": "agent", "type": "deploy_agent", "env_ids": [], + "a2a_agent_id": "claude-code"}])} + + registered = register_bundle(layout(home / "triage", files)) + + written = {done.write.id: done.version for done in registered.materialization.writes} + assert written == {"claude-code__agent_image": 1, "claude-code": 1, "triage-t": 1} + step = _steps("triage-t")[1] + assert (step["a2a_agent_id"], step["a2a_agent_version"]) == ("claude-code", 1) + image = DockerImageArtifact.get("claude-code__agent_image") + assert (image.image_name, image.image_digest) == (agents_published, DIGEST) + assert A2AAgent.get("claude-code").docker_image_artifact.id == "claude-code__agent_image" + + +def test_an_image_the_bundle_builds_is_pushed_and_registered_by_its_digest_with_its_context(home, monkeypatch): + pushed = [] + + def push(entity_id, context_path, dockerfile_path, before=None): + pushed.append((entity_id, dockerfile_path)) + return f"registry.example.com/{entity_id}:src-1", DIGEST + + monkeypatch.setattr(materialize_module, "push_context_image", push) + files = {"envs/crm/Dockerfile": "FROM python:3.12-slim\n", "envs/crm/env.toml": 'environment_name = "crm"\n', + "tasks/t.json": json.dumps([{"id": "env", "type": "deploy_env", "env_id": "crm"}])} + + register_bundle(layout(home / "triage", files)) + + assert [(entity_id, Path(dockerfile).name) for entity_id, dockerfile in pushed] == [ + ("triage-crm__env_image", "Dockerfile")] + image = DockerImageArtifact.get("triage-crm__env_image") + assert (image.image_name, image.image_digest) == ("registry.example.com/triage-crm__env_image:src-1", DIGEST) + assert image.build_context_object_url and image.dockerfile_path == "Dockerfile" + assert Env.get("triage-crm").docker_image_artifact.id == "triage-crm__env_image" + step = _steps("triage-t")[0] + assert (step["env_id"], step["env_version"]) == ("triage-crm", 1) + + +def test_an_at_local_id_left_in_what_would_be_written_is_refused(home): + files = {"tasks/t.json": json.dumps([{**BOX, "metadata": {"from": "@local/~/elsewhere/x"}}])} + + with pytest.raises(BundleError) as refused: + register_bundle(layout(home / "triage", files)) + + assert refused.value.problems == ( + "tasks/t.json: [0].metadata.from names '@local/~/elsewhere/x', which only this machine's stores hold",) + + +def test_a_shared_store_refuses_a_credential_written_out_and_a_url_only_this_machine_reaches(home, monkeypatch): + monkeypatch.setattr(register_module, "shared_store", lambda store: True) + files = {"tasks/t.json": json.dumps([ + {**BOX, "env_vars": {"API_KEY": "abc", "TOKEN": "secret:team_token", "MODE": "fast"}}, + {"id": "agent", "type": "deploy_agent", "env_ids": [], "metadata": {"wiki": "http://localhost:8080/x"}}])} + + with pytest.raises(BundleError) as refused: + register_bundle(layout(home / "triage", files)) + + assert refused.value.problems == ( + "tasks/t.json: [0].env_vars.API_KEY: its name looks like a secret's, and it's set, and a shared store shows " + "it to anyone who reads it; leave it out, for whatever runs the task to give it", + "tasks/t.json: [1].metadata.wiki names http://localhost:8080, which only this machine reaches") + + +def test_the_cli_says_what_it_would_write_asks_before_a_shared_store_and_dry_run_writes_nothing(home, monkeypatch): + root = layout(home / "triage", TRIAGE) + monkeypatch.setattr(register_module, "shared_store", lambda store: True) + + dry = CliRunner().invoke(cli, ["bundle", "register", str(root), "--dry-run"]) + declined = CliRunner().invoke(cli, ["bundle", "register", str(root)], input="n\n") + written = CliRunner().invoke(cli, ["bundle", "register", str(root), "--yes"]) + again = CliRunner().invoke(cli, ["bundle", "register", str(root), "--yes"]) + + assert dry.exit_code == 0, dry.output + assert " artifacts/docs -> artifact triage-docs v1 (new)" in dry.output + assert dry.output.rstrip().endswith("Nothing was written: --dry-run.") + assert declined.exit_code == 1 and "Write these to the stores above?" in declined.output + assert written.exit_code == 0, written.output + assert "Run a registered task with: agent-env task run --id triage-t --version 1" in written.output + assert "Nothing to write: every version is registered already." in again.output + assert [task.version for task in [Task.get("triage-t")]] == [1] + + +def test_a_bundle_with_no_tasks_registers_every_entity(home): + registered = register_bundle(layout(home / "library", {"artifacts/greeting/hello.txt": "hello\n"})) + + assert [done.write.id for done in registered.materialization.writes] == ["library-greeting"] + + +def test_a_failed_write_keeps_what_was_written_and_the_next_run_reuses_it(home, monkeypatch): + root = layout(home / "triage", TRIAGE) + real = Task.put + + def failing(*args, **kwargs): + raise RuntimeError("the store went away") + + monkeypatch.setattr(Task, "put", failing) + with pytest.raises(RuntimeError): + register_bundle(root) + monkeypatch.setattr(Task, "put", real) + + retried = register_bundle(root) + + assert {done.write.id: (done.version, done.reused) for done in retried.materialization.writes} == { + "triage-docs": (1, True), "triage-greeting": (1, True), "triage-t": (1, False)} + assert Artifact.get("triage-docs").version == 1 + + +class _Store: + """An image store that records each repository it's asked to set up.""" + + def __init__(self): + self.repositories = [] + + def image_ref(self, repository, tag): + return f"registry.example.com/{repository}:{tag}" + + def ensure_repository(self, repository): + self.repositories.append(repository) + + def auth(self, ref): + return None + + +def test_an_image_is_built_and_pushed_once_under_the_tag_of_its_sources(home, monkeypatch): + store, built, pushed, served = _Store(), [], [], set() + + def pin(ref, auth): + if ref not in served: + raise docker_image_module.NoSuchImageError(f"{ref}: no such image") + return f"{ref}@{DIGEST}" + + monkeypatch.setattr(get_config(), "get_image_store_for", lambda entity_id: store) + monkeypatch.setattr(docker_image_module, "pin_digest", pin) + monkeypatch.setattr(docker_image_module, "build_image", + lambda dockerfile, context, tag, platform: built.append((tag, platform))) + monkeypatch.setattr(docker_image_module, "_push_local_image", + lambda local, ref, image_store: (pushed.append(ref), served.add(ref))) + context = layout(home / "context", {"Dockerfile": "FROM python:3.12-slim\n"}) + + first = push_context_image("triage-crm__env_image", str(context), "Dockerfile") + again = push_context_image("triage-crm__env_image", str(context), "Dockerfile") + + ref = first[0] + assert first == again == (ref, DIGEST) + assert ref.startswith("registry.example.com/triage-crm-env_image:src-") and "__" not in ref + assert [platform for _, platform in built] == ["linux/amd64"] and pushed == [ref] + assert store.repositories == ["triage-crm-env_image", "triage-crm-env_image"] From 707d7002253e63abd47c0cbafc06dc67fbd18bf0 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 11:58:30 -0700 Subject: [PATCH 03/22] fix(bundle): a registration that loses a race for a version fails rather than write the next one Two registrations of one bundle on different machines aren't serialized by the local locks, so both wrote every id, the second at v2. The registration ledger now holds each write to the version its check found next (expecting_version): a put of that id then takes exactly that version, through insert_next, or raises ConcurrentModificationError, and a rerun reuses what the winner wrote. Only an agent an installed bundle comes with is registered under its alias: no store env may start on an installed env alias's id, so an installed env is refused before anything is written. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- src/agent_env/artifact/store.py | 4 ++ src/agent_env/bundle/ledger.py | 10 ++-- src/agent_env/bundle/register.py | 18 ++++--- .../store/document_store/__init__.py | 4 ++ .../store/document_store/document_store.py | 30 +++++++++-- tst/unit/bundle/register_test.py | 53 ++++++++++++++++++- 7 files changed, 105 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 651af9ade..7004a1db8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,7 +114,7 @@ and deserialized through a registry. | `a2a_agent/` | The `A2AAgent` entity (`a2a_agent`), its stores and the validator steps. The protocol package provides the agent-side framework. | | `openenv/` | `build_app(bundle)`: the [OpenEnv](https://github.com/openenv/OpenEnv) server for a bundle's tasks, each session playing one episode through the Episode API, with the envs' MCP tools, an added `end_turn` tool and the task's verifier score as the reward. It prepares bundles as `agent-env run` does (`prepare_bundle(played=True)`, without the agent the client plays) and keeps namespace routing on for the app's life. `openenv` comes with the `openenv` extra, a floor with no cap, and this package is the only one that imports it. | | `runner/` | The `[runner]` seam: `Runner.submit()` returns a `RunHandle` (`run_id`, `instance_id`) before the run starts; `LocalRunner` is built in. `runner/store.py` is the run index every runner writes and the explorer reads. | -| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store. | +| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | | `eval/` | `Eval` (`eval`), a versioned list of task references, and its store. | | `plugins/` | Entry-point plugins (see Extension points): the group names, `inventory()`, which `agent-env plugin list/show/check` report, `load_failures` and `settings`. The top level is public; the underscored modules are internal. | | `explorer/` | Optional local web UI: `agent-env up`, binds loopback `:8234`. | diff --git a/src/agent_env/artifact/store.py b/src/agent_env/artifact/store.py index 716fd01aa..a574afa1b 100644 --- a/src/agent_env/artifact/store.py +++ b/src/agent_env/artifact/store.py @@ -22,6 +22,7 @@ UpdateSpec, VersionedEntityStore, VersionedEntityStoreCache, + expected_version, ) from agent_env.store.ids import key_segment from agent_env.store.query import QueryBuilder, to_document_query @@ -193,6 +194,9 @@ def put_document(self, artifact: _A) -> _A: if artifact.version > 0: versioned.insert_next(doc, artifact.version) return artifact.model_copy() + if (expected := expected_version(artifact.id)) is not None: + versioned.insert_next(doc, expected) + return artifact.model_copy(update={"version": expected}) version = versioned.next_version(artifact.id) last_exc: Optional[DuplicateKeyError] = None for attempt in range(_MAX_VERSION_RETRIES): diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index a6e3c9dba..7f05d47fc 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -13,7 +13,7 @@ import json import stat from collections.abc import Callable, Iterable, Iterator, Mapping -from contextlib import contextmanager +from contextlib import contextmanager, nullcontext from dataclasses import dataclass from datetime import date, datetime, time, timezone from pathlib import Path @@ -27,7 +27,7 @@ from agent_env.env.store import ENVS_COLLECTION from agent_env.eval.store import EVALS_COLLECTION from agent_env.store import Filter, Sort -from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore +from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore, expecting_version from agent_env.store.local_state import holding_locks from agent_env.task.store import TASKS_COLLECTION @@ -82,6 +82,9 @@ class Ledger: collection = LEDGER_COLLECTION # Whether a task or eval is written naming the versions of what it references, so is hashed with them. pins_references = False + # Whether each write lands on exactly the version its check found next, so one that loses a race to another + # writer raises ConcurrentModificationError rather than add a version after the other's. + strict = False def __init__(self, store: DocumentStore, plan: Plan): self._store = store @@ -157,7 +160,8 @@ def record(self, check: Check, write: Callable[[], int]) -> int: "digest": check.digest.value if check.digest else None, "inputs": check.digest.inputs if check.digest else None, "at": _now(), **self.row_fields()} self._store.insert(self.collection, pending) - version = write() + with expecting_version(check.write.id, check.next_version) if self.strict else nullcontext(): + version = write() # Stamped before the check below, which can take seconds, so a run interrupted during it leaves the version # this attempt wrote, and the next run keeps it rather than writing it again. self._store.replace(self.collection, Filter.of(**key, status="pending"), {**pending, "version": version}) diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index cf8739af2..da813e22f 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -30,7 +30,7 @@ from agent_env.config import get_config from agent_env.entity.refs import RefRole, ref_sites -from agent_env.plugins._bundles import AGENT_ALIASES, ENV_ALIASES +from agent_env.plugins._bundles import AGENT_ALIASES from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore from agent_env.store.ids import LOCAL_PREFIX, is_local_id from agent_env.store.image_store.oci_registry_credentials import is_loopback_host @@ -146,8 +146,9 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: """The registry id each ``@local`` id the plan writes takes, and each problem giving them. The bundle's own entry is ````, its name lowercased with each run of what an id can't hold made - ``-``. An installed bundle's agent or env pulled in by a reference takes the first alias its bundle gives it, - which the reference named it by. An id derived from another (``__env_image``) keeps its suffix after the + ``-``. An installed bundle's agent pulled in by a reference takes the first alias its bundle gives it, which the + reference named it by; any other entity of an installed bundle has no registry id, and an env's alias can't be + one, since no store env may start on an installed env alias's id. An id derived from another (``__env_image``) keeps its suffix after the other's registry id, and a task's output is ``-``.""" ids: dict[str, str] = {} problems: list[str] = [] @@ -158,8 +159,8 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: if entry in plan.origins: alias = _alias(plan.origins[entry], entry) if alias is None: - problems.append(f"{plan.path(entry)}: comes with an installed bundle and has no alias to register it " - "under; name an entity the store holds instead") + problems.append(f"{plan.path(entry)}: comes with an installed bundle, and only an agent with an alias " + "is registered from one; name an entity the store holds instead") continue ids[entry.id] = alias elif name := _slug(entry.name): @@ -186,8 +187,7 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: def _alias(bundle: Bundle, entry: BundleEntry) -> str | None: - table = {BundleKind.AGENT: AGENT_ALIASES, BundleKind.ENV: ENV_ALIASES}.get(entry.kind) - aliases = bundle.aliases.get(table, {}).get(entry.name, ()) if table else () + aliases = bundle.aliases.get(AGENT_ALIASES, {}).get(entry.name, ()) if entry.kind is BundleKind.AGENT else () return aliases[0] if aliases else None @@ -269,10 +269,12 @@ def _renamed(value: Any, ids: Mapping[str, str]) -> Any: class RegistrationLedger(Ledger): """A registration's rows, in the target store: each names the bundle that wrote it, by its identity, and the commit - and state of the folder it was written from. A task or eval is hashed with the versions it names.""" + and state of the folder it was written from. A task or eval is hashed with the versions it names, and each write + lands on exactly the version its check found next.""" collection = REGISTRATIONS pins_references = True + strict = True def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None): super().__init__(store, plan) diff --git a/src/agent_env/store/document_store/__init__.py b/src/agent_env/store/document_store/__init__.py index 0ec038e3d..1ca2ed869 100644 --- a/src/agent_env/store/document_store/__init__.py +++ b/src/agent_env/store/document_store/__init__.py @@ -31,6 +31,8 @@ VersionedEntityStore, VersionedEntityStoreCache, compare_and_swap, + expected_version, + expecting_version, reject_reserved_id, rev_precondition, ) @@ -46,6 +48,8 @@ "VersionedEntityStore", "VersionedEntityStoreCache", "compare_and_swap", + "expected_version", + "expecting_version", "reject_reserved_id", "rev_precondition", "Filter", diff --git a/src/agent_env/store/document_store/document_store.py b/src/agent_env/store/document_store/document_store.py index 1849a15ca..9c2f56a11 100644 --- a/src/agent_env/store/document_store/document_store.py +++ b/src/agent_env/store/document_store/document_store.py @@ -5,6 +5,9 @@ import json import logging from abc import ABC, abstractmethod +from collections.abc import Iterator, Mapping +from contextlib import contextmanager +from contextvars import ContextVar from dataclasses import dataclass, field from typing import Any, Callable, Generic, Optional, TypeVar, Union @@ -97,6 +100,26 @@ def where(self, field_path: str, predicate: Predicate) -> "Filter": # --- Sort ------------------------------------------------------------------ + +_expected_versions: ContextVar[Mapping[str, int]] = ContextVar("agent_env_expected_versions", default={}) + + +@contextmanager +def expecting_version(entity_id: str, version: int) -> Iterator[None]: + """Inside the block, a put of ``entity_id`` that names no version writes exactly ``version``, which must be the + id's next, as one that names it does: a write that loses a race to another raises ConcurrentModificationError + rather than land on the version after the other's.""" + token = _expected_versions.set({**_expected_versions.get(), entity_id: version}) + try: + yield + finally: + _expected_versions.reset(token) + + +def expected_version(entity_id: str) -> Optional[int]: + """The version ``expecting_version`` holds a put of ``entity_id`` to, or None.""" + return _expected_versions.get().get(entity_id) + @dataclass(frozen=True) class SortKey: field: str @@ -465,9 +488,10 @@ def put(self, entity: T, max_retries: int = 5) -> int: if not isinstance(doc.get("id"), str): raise ValueError(f"an entity in {self._collection!r} needs a string id, got {doc.get('id')!r}") self._doc_store.check_id(doc["id"]) - if doc.get("version"): - self.insert_next(doc, doc["version"]) - return doc["version"] + version = doc.get("version") or expected_version(doc["id"]) + if version: + self.insert_next(doc, version) + return version last_error: Optional[DuplicateKeyError] = None for _ in range(max_retries): version = self.next_version(doc["id"]) diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index e68aa109e..5b7fb98e4 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -23,7 +23,10 @@ from agent_env.entity.refs import EntityRef, RefRole from agent_env.env.env import Env from agent_env.eval import Eval -from agent_env.store import Filter +from agent_env.bundle.parse import BundleKind +from agent_env.store import ConcurrentModificationError, Filter +from agent_env.store.base import NotFoundError +from agent_env.store.document_store import expecting_version from agent_env.task import Task from agent_env.task_step.task_step import TaskStep from tst.unit.bundle._support import DIGEST, layout @@ -394,3 +397,51 @@ def pin(ref, auth): assert ref.startswith("registry.example.com/triage-crm-env_image:src-") and "__" not in ref assert [platform for _, platform in built] == ["linux/amd64"] and pushed == [ref] assert store.repositories == ["triage-crm-env_image", "triage-crm-env_image"] + + +def test_a_put_held_to_a_version_writes_exactly_it_or_fails(home): + (home / "seed.txt").write_text("seed") + + with expecting_version("held", 1): + held = FileArtifact.put("held", description="first", file_path=str(home / "seed.txt")) + with expecting_version("held", 1), pytest.raises(ConcurrentModificationError): + FileArtifact.put("held", description="late", file_path=str(home / "seed.txt")) + with expecting_version("held-task", 2), pytest.raises(ConcurrentModificationError): + Task.put(id="held-task", steps=[]) + + assert held.version == 1 and Artifact.get("held").description == "first" + + +def test_a_registration_that_loses_a_race_for_a_version_fails_rather_than_write_the_next_one(home, monkeypatch): + """As a registration of the same bundle on another machine, which no local lock waits for, would.""" + root = layout(home / "triage", TRIAGE) + writer = materialize_module._WRITERS[BundleKind.ARTIFACT] + + def raced(plan, write): + if write.id == "triage-docs" and not raced.done: + raced.done = True + (home / "other.txt").write_text("the other machine's") + with expecting_version("triage-docs", 1): + FileArtifact.put("triage-docs", description="won the race", file_path=str(home / "other.txt")) + return writer(plan, write) + + raced.done = False + monkeypatch.setitem(materialize_module._WRITERS, BundleKind.ARTIFACT, raced) + + with pytest.raises(ConcurrentModificationError): + register_bundle(root) + + assert Artifact.get("triage-docs", 1).description == "won the race" + with pytest.raises(NotFoundError): + Artifact.get("triage-docs", 2) + + +def test_an_installed_env_a_task_names_by_its_alias_is_refused_before_anything_is_written(home): + files = {"tasks/t.json": json.dumps([{"id": "env", "type": "deploy_env", "env_id": "mail"}])} + + with pytest.raises(BundleError) as refused: + register_bundle(layout(home / "triage", files)) + + assert "comes with an installed bundle, and only an agent with an alias is registered from one" in \ + refused.value.problems[0] + assert get_config().get_document_store().count("envs", Filter()) == 0 From cf01e9417d436806bfabda620d3575a7d9137e3a Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 12:08:28 -0700 Subject: [PATCH 04/22] fix(bundle): a write that fails leaves no pending ledger row A pending row marks a write a run started, so the next run keeps the version an interrupted one wrote. One whose write raised wrote no version the run knows of, so the row is removed: a registration that lost a race left one behind in the shared store. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/ledger.py | 9 +++++++-- tst/unit/bundle/register_test.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 7f05d47fc..20a180caf 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -160,8 +160,13 @@ def record(self, check: Check, write: Callable[[], int]) -> int: "digest": check.digest.value if check.digest else None, "inputs": check.digest.inputs if check.digest else None, "at": _now(), **self.row_fields()} self._store.insert(self.collection, pending) - with expecting_version(check.write.id, check.next_version) if self.strict else nullcontext(): - version = write() + try: + with expecting_version(check.write.id, check.next_version) if self.strict else nullcontext(): + version = write() + except Exception: + # It wrote no version this run knows of, so the row marks nothing for the next run to keep. + self._store.delete(self.collection, Filter.of(**key, status="pending")) + raise # Stamped before the check below, which can take seconds, so a run interrupted during it leaves the version # this attempt wrote, and the next run keeps it rather than writing it again. self._store.replace(self.collection, Filter.of(**key, status="pending"), {**pending, "version": version}) diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 5b7fb98e4..5b147e29b 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -445,3 +445,14 @@ def test_an_installed_env_a_task_names_by_its_alias_is_refused_before_anything_i assert "comes with an installed bundle, and only an agent with an alias is registered from one" in \ refused.value.problems[0] assert get_config().get_document_store().count("envs", Filter()) == 0 + + +def test_a_write_that_fails_leaves_no_pending_row(home, monkeypatch): + def failing(*args, **kwargs): + raise RuntimeError("the store went away") + + monkeypatch.setattr(Task, "put", failing) + with pytest.raises(RuntimeError): + register_bundle(layout(home / "triage", TRIAGE)) + + assert get_config().get_document_store().query(REGISTRATIONS, Filter.of(status="pending")) == [] From 229f927892d1bcbd899448f745b3d725f6b1379f Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 12:49:36 -0700 Subject: [PATCH 05/22] fix(bundle): pin each reference by its own field, resume an interrupted registration, recheck ownership at each write - A step naming no version took another step's pin of the same id; each now takes the version the bundle wrote, or the store's latest the plan read. - A registration killed after a write but before its ledger row was done couldn't resume: the rerun refused the id as unowned. A pending row of this bundle at the stored version now counts as its own, so the ledger adopts it. - Ownership is checked again as each write is checked, so an id another bundle registers after the first check is refused before it's written, not taken at its next version. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/ledger.py | 4 -- src/agent_env/bundle/materialize.py | 16 +++++--- src/agent_env/bundle/register.py | 57 ++++++++++++++++---------- tst/unit/bundle/register_test.py | 62 +++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 30 deletions(-) diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 20a180caf..d97bcc852 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -188,10 +188,6 @@ def row_fields(self) -> dict[str, Any]: """What each row records beyond the write: nothing here.""" return {} - def latest_row(self, store: str, id: str) -> dict | None: - """The latest done row for (``store``, ``id``), whichever bundle wrote it.""" - return self._latest(store, id) - def _orphan(self, write: Write, stored: int | None, digest: Digest) -> int | None: """The version an interrupted run of this bundle wrote and didn't record, when it's still the store's latest and was made from what ``write`` is now made from.""" diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 6e9f0c1cc..7f42c5049 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -378,16 +378,20 @@ def _build(write: Write, plan: Plan, versions: dict[tuple[str, str], int] | None def pinned_steps(plan: Plan, write: Write, versions: dict[tuple[str, str], int]) -> list[dict]: - """A copy of the task ``write``'s steps with each reference it reads, the bundle's own entity or a store's, naming - the version it reads (``_version``), where its field takes a version.""" + """A copy of the task ``write``'s steps with each reference it reads that names no version, where its field takes + one, naming the version it reads: the one written for the bundle's own entity, or the store's latest the plan + read.""" steps = copy.deepcopy(write.source.config) - references = {(ref.kind, ref.id): ref for ref in write.source.references} + own = {(ref.kind, ref.id) for ref in write.source.references if ref.local is not None} registry = get_task_step_registry() for step in steps: for site in ref_sites(registry[step["type"]].entity_refs or (), step): - ref = references.get((site.ref.kind, site.value)) - if site.ref.role is RefRole.INPUT and site.version_key is not None and site.version is None and ref: - site.rewrite(site.value, _version(plan, ref, versions)) + if site.ref.role is not RefRole.INPUT or site.version_key is None or site.version is not None: + continue + key = (site.ref.kind, site.value) + version = versions[_KINDS[site.ref.kind], site.value] if key in own else plan.store_latest.get(key) + if version is not None: + site.rewrite(site.value, version) return steps diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index da813e22f..b2838d6e0 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -30,6 +30,7 @@ from agent_env.config import get_config from agent_env.entity.refs import RefRole, ref_sites +from agent_env.store import Filter from agent_env.plugins._bundles import AGENT_ALIASES from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore from agent_env.store.ids import LOCAL_PREFIX, is_local_id @@ -39,7 +40,7 @@ from ._secrets import credentials_in from .installed import installed_bundles -from .ledger import Ledger +from .ledger import Check, Ledger from .materialize import Materialization, Materialized, Registering, materialize from .parse import Bundle, BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle, plan_entities @@ -94,8 +95,8 @@ def register_bundle( target = configured_store(get_config().get_document_store()) shared = shared_store(target) identity, commit, dirty = bundle_identity(bundle.root) - ledger = RegistrationLedger(target, registered, identity, commit, dirty) - problems = [*_unowned(registered, ledger, frozenset(adopt)), *_unregistrable(registered, shared)] + ledger = RegistrationLedger(target, registered, identity, commit, dirty, frozenset(adopt)) + problems = [*_unowned(registered, ledger), *_unregistrable(registered, shared)] if problems: raise BundleError(problems) say = on_progress or (lambda line: None) @@ -276,12 +277,35 @@ class RegistrationLedger(Ledger): pins_references = True strict = True - def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None): + def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None, + adopt: frozenset[str] = frozenset()): super().__init__(store, plan) - self.identity, self.commit, self.dirty = identity, commit, dirty - - def bundle_key(self, write: Write) -> str: - return self._bundle(write) + self.identity, self.commit, self.dirty, self.adopted_ids = identity, commit, dirty, adopt + + def check(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Check: + """``Ledger.check``, refusing a write whose id the store holds that this bundle didn't register, checked again + as it's about to be written, since another bundle can write the id after the check before any write.""" + check = super().check(write, needs) + if (problem := self.unowned(write, check.stored)) is not None: + raise BundleError([problem]) + return check + + def unowned(self, write: Write, stored: int | None) -> str | None: + """Why this bundle may not write ``write``'s id, which the store holds at ``stored``, or None when it may: the + store doesn't hold it, ``adopt`` names it, this bundle registered its latest version, or an interrupted + registration of this bundle wrote it.""" + if stored is None or write.id in self.adopted_ids: + return None + row = self._latest(write.kind.store, write.id) + if row is not None and row.get("bundle") == self._bundle(write): + return None + pending = self._find(self.collection, Filter.of(store=write.kind.store, id=write.id, bundle=self._bundle(write), + status="pending")) + if pending is not None and pending.get("version") == stored: + return None + by = f"registered from {row['bundle']}" if row is not None else "not registered from a bundle" + return (f"{self._plan.path(write.source.entry)}: {write.id!r} is in the store already, at v{stored}, {by}; pass " + f"--adopt {write.id} to write a new version of it, or --prefix to register this bundle under other ids") def stored_version(self, write: Write) -> int | None: return self._stored_version(write) @@ -327,19 +351,12 @@ def _remote_name(remote: str) -> str: return f"{host}/{path.strip('/').removesuffix('.git')}" -def _unowned(plan: Plan, ledger: RegistrationLedger, adopt: frozenset[str]) -> Iterator[str]: - """A problem for each write whose id the store holds already, unless this bundle registered it or ``adopt`` names - it.""" +def _unowned(plan: Plan, ledger: RegistrationLedger) -> Iterator[str]: + """A problem for each write this bundle may not make (``RegistrationLedger.unowned``), every one found before any + write.""" for write in plan.writes: - stored = ledger.stored_version(write) - if stored is None or write.id in adopt: - continue - row = ledger.latest_row(write.kind.store, write.id) - if row is not None and row.get("bundle") == ledger.bundle_key(write): - continue - by = f"registered from {row['bundle']}" if row is not None else "not registered from a bundle" - yield (f"{plan.path(write.source.entry)}: {write.id!r} is in the store already, at v{stored}, {by}; pass " - f"--adopt {write.id} to write a new version of it, or --prefix to register this bundle under other ids") + if (problem := ledger.unowned(write, ledger.stored_version(write))) is not None: + yield problem def _unregistrable(plan: Plan, shared: bool) -> Iterator[str]: diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 5b147e29b..6ca71b66a 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -456,3 +456,65 @@ def failing(*args, **kwargs): register_bundle(layout(home / "triage", TRIAGE)) assert get_config().get_document_store().query(REGISTRATIONS, Filter.of(status="pending")) == [] + + +def test_a_step_naming_no_version_takes_the_latest_even_where_another_step_pins_one(home): + (home / "seed.txt").write_text("seed") + FileArtifact.put("shared-seed", description="seed", file_path=str(home / "seed.txt")) + FileArtifact.put("shared-seed", description="seed", file_path=str(home / "seed.txt")) + files = {"tasks/t.json": json.dumps([BOX, _load("latest", "shared-seed"), + {**_load("pinned", "shared-seed"), "artifact_version": 1}])} + + register_bundle(layout(home / "triage", files)) + + assert [step["artifacts"] for step in _steps("triage-t")[1:]] == [[{"id": "shared-seed", "version": 2}], + [{"id": "shared-seed", "version": 1}]] + + +class _Killed(BaseException): + """Stands in for the process dying.""" + + +def test_a_registration_killed_after_a_write_and_before_its_row_is_done_resumes(home, monkeypatch): + root = layout(home / "triage", TRIAGE) + digest = register_module.Ledger.digest + calls = [] + + def dies_after_writing_docs(self, write, needs): + calls.append(write.id) + if write.id == "triage-docs" and calls.count("triage-docs") == 2: # the check's, then the one after the write + raise _Killed + return digest(self, write, needs) + + monkeypatch.setattr(register_module.Ledger, "digest", dies_after_writing_docs) + with pytest.raises(_Killed): + register_bundle(root) + monkeypatch.setattr(register_module.Ledger, "digest", digest) + + resumed = register_bundle(root) + + assert {done.write.id: (done.version, done.reused) for done in resumed.materialization.writes} == { + "triage-docs": (1, True), "triage-greeting": (1, False), "triage-t": (1, False)} + + +def test_an_id_another_bundle_registers_after_the_check_is_refused_before_it_is_written(home, monkeypatch): + root = layout(home / "triage", TRIAGE) + unowned = register_module._unowned + + def then_another_bundle_writes(plan, ledger): + problems = list(unowned(plan, ledger)) + (home / "theirs.txt").write_text("theirs") + FileArtifact.put("triage-docs", description="theirs", file_path=str(home / "theirs.txt")) + get_config().get_document_store().insert(REGISTRATIONS, { + "store": "artifact", "id": "triage-docs", "bundle": "folder:/elsewhere/triage", "status": "done", + "version": 1, "scheme": 2, "digest": None, "inputs": None}) + return iter(problems) + + monkeypatch.setattr(register_module, "_unowned", then_another_bundle_writes) + + with pytest.raises(BundleError) as refused: + register_bundle(root) + + assert "'triage-docs' is in the store already, at v1, registered from folder:/elsewhere/triage" in \ + refused.value.problems[0] + assert Artifact.get("triage-docs").description == "theirs" From 85adc0aad73e8326efadd605da05d300bd8b33a0 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 13:37:37 -0700 Subject: [PATCH 06/22] feat(bundle): register a folder bundle init wrote back into the store it came from When the folder's provenance.json records an import from the configured store: - each entity, image, task and eval takes the store id it was imported from, and an imported task's outputs the names its file gives them; - a write the folder still holds as the import wrote it, with all it needs at the versions they were imported at, keeps the version it was imported at, so an unedited folder writes nothing, and the ledger records it; - an edit re-versions only what it reaches; - each imported task's steps get back the user-scoped fields the import left out (its agent, env vars, sandbox type...), from the stored version, by step id, where the folder doesn't set them. A shared store's credential and URL checks skip those fields, which it holds already. A folder imported from another store, or recorded by an older agent-env, registers as a new bundle, and says why. Registering over a version someone else wrote since, the bundle's own or the imported one, is refused unless --adopt. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_imported.py | 151 +++++++++++++++++++++ src/agent_env/bundle/register.py | 127 ++++++++++++++---- tst/unit/bundle/register_imports_test.py | 159 +++++++++++++++++++++++ tst/unit/bundle/register_test.py | 16 +++ 4 files changed, 429 insertions(+), 24 deletions(-) create mode 100644 src/agent_env/bundle/_imported.py create mode 100644 tst/unit/bundle/register_imports_test.py diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py new file mode 100644 index 000000000..7d59463e6 --- /dev/null +++ b/src/agent_env/bundle/_imported.py @@ -0,0 +1,151 @@ +"""What a folder ``bundle init`` wrote holds of the store it was imported from, for registering it back: each entity, +image, task and eval's store id and version, whether the folder still holds each as the import wrote it, and the +user-scoped fields the import left out of each task's steps.""" + +from __future__ import annotations + +import copy +import json +import unicodedata +from collections.abc import Mapping +from dataclasses import dataclass, replace +from pathlib import Path + +from agent_env.entity.paths import field_sites +from agent_env.entity.scopes import FieldScope +from agent_env.env.registry import get_env_registry +from agent_env.store.base import NotFoundError +from agent_env.task.task import Task +from agent_env.task_step.registry import get_task_step_registry + +from ._import_report import PROVENANCE, PROVENANCE_SCHEMA, store_fingerprint +from .parse import CONFIG_FILES, Bundle, BundleEntry, BundleKind +from .plan import Plan, Write, env_folder_writer +from .resolve import BuiltImage, BundleImage + + +@dataclass(frozen=True) +class Imported: + """What an import recorded: each entity folder, task file and eval file's store id and version, each image's by its + folder and toml key, and each file as it wrote it.""" + + entities: Mapping[str, tuple[str, int]] + images: Mapping[tuple[str, str], tuple[str, int]] + files: Mapping[str, str] + + +def read_imported(root: Path) -> tuple[Imported | None, str | None]: + """What the import of ``root`` recorded, when it was imported from the configured store; else None, and why when + it has a record this can't use.""" + path = root / PROVENANCE + if not path.is_file(): + return None, None + try: + record = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return None, f"{PROVENANCE} can't be read, so the folder registers as a new bundle" + if not isinstance(record, dict) or record.get("schema") != PROVENANCE_SCHEMA: + return None, f"{PROVENANCE} is from an older agent-env, so the folder registers as a new bundle" + if record.get("store") != store_fingerprint(): + return None, "it was imported from another store, so its entries take new ids" + entities = {item["folder"]: (item["id"], item["version"]) for item in record.get("written", [])} + entities |= {item["file"]: (item["id"], item["version"]) for item in record.get("tasks", [])} + if (stored_eval := record.get("eval")) is not None: + entities[stored_eval["file"]] = (stored_eval["id"], stored_eval["version"]) + images = {(item["folder"], item["key"]): (item["id"], item["version"]) for item in record.get("images", [])} + return Imported(entities, images, record.get("files", {})), None + + +def where(root: Path, entry: BundleEntry) -> str: + """``entry``'s folder or file in the bundle at ``root``, as the import's record names it.""" + return unicodedata.normalize("NFC", entry.path.relative_to(root).as_posix()) + + +def source_of(imported: Imported, plan: Plan, write: Write) -> tuple[str, int] | None: + """The store id and version ``write`` was imported from, or None when the import didn't write it.""" + entry = write.source.entry + folder = where(plan.bundle_of(entry).root, entry) + if isinstance(write.source, BundleImage) and write.source.id != entry.id: + return imported.images.get((folder, image_key(write.source))) + return imported.entities.get(folder) + + +def image_key(image: BundleImage) -> str: + """The toml key an image an env or agent folder builds is named by: ``image`` for its own, as the import names it.""" + role = image.id[len(image.entry.id) + 2:] + return "image" if role == f"{image.entry.kind.store}_image" else role + + +def as_imported(imported: Imported, plan: Plan, write: Write, now: Mapping[str, str]) -> bool: + """Whether the files ``write`` is made from are those the import wrote: a task's or eval's file; an image's build + context, its folder but the toml; an env's or agent's toml, or every file of an env type written from its folder; + every file of an artifact or skill folder.""" + entry = write.source.entry + folder = where(plan.bundle_of(entry).root, entry) + if write.kind in (BundleKind.TASK, BundleKind.EVAL): + return folder in imported.files and imported.files[folder] == now.get(folder) + toml = f"{folder}/{CONFIG_FILES[entry.kind]}" if entry.kind in CONFIG_FILES else None + then, current = _within(imported.files, folder), _within(now, folder) + if isinstance(write.source, BuiltImage): + then.pop(toml, None) + current.pop(toml, None) + elif isinstance(write.source, BundleImage) or (entry.kind in (BundleKind.ENV, BundleKind.AGENT) and not ( + entry.kind is BundleKind.ENV and _written_from_folder(entry))): + then, current = {toml: then.get(toml)}, {toml: current.get(toml)} + return then == current + + +def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]]]: + """``bundle`` with each imported task's steps given back the user-scoped fields the import left out, from the + stored version it was imported from, matched by step id and type, where the folder doesn't set them; and, for each + task's file, the paths in its steps that took one.""" + registry = get_task_step_registry() + entries, taken = [], {} + for entry in bundle.entries: + source = imported.entities.get(where(bundle.root, entry)) if entry.kind is BundleKind.TASK else None + try: + stored = Task.get(*source).to_dict()["steps"] if source is not None else None + except NotFoundError: + stored = None + if stored is None: + entries.append(entry) + continue + by_id = {step.get("id"): step for step in stored} + steps = copy.deepcopy(entry.config) + for index, step in enumerate(steps): + origin = by_id.get(step.get("id")) + cls = registry.get(step.get("type")) + if origin is None or cls is None or origin.get("type") != step.get("type"): + continue + for path, scope in cls.field_scopes.items(): + if scope is not FieldScope.USER or "[]" in path: + continue + for _, holder, key in field_sites(origin, path): + if holder[key] not in ("", [], {}) and _graft(step, path.split("."), holder[key]): + taken.setdefault(entry.path, set()).add(f"[{index}].{path}") + entries.append(replace(entry, config=steps)) + return replace(bundle, entries=tuple(entries)), taken + + +def _graft(step: dict, keys: list[str], value: object) -> bool: + """Set ``keys`` in ``step`` to ``value`` when it's unset there; whether it was.""" + holder = step + for key in keys[:-1]: + child = holder.get(key) + if child is None: + child = holder[key] = {} + if not isinstance(child, dict): + return False + holder = child + if holder.get(keys[-1]) is not None: + return False + holder[keys[-1]] = copy.deepcopy(value) + return True + + +def _within(files: Mapping[str, str], folder: str) -> dict[str, str]: + return {path: digest for path, digest in files.items() if path.startswith(f"{folder}/")} + + +def _written_from_folder(entry: BundleEntry) -> bool: + return env_folder_writer(get_env_registry().get(entry.type)) diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index b2838d6e0..e1b685aff 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -38,6 +38,8 @@ from agent_env.store.routing import configured_store, namespace_routing from agent_env.task_step.registry import get_task_step_registry +from ._import_report import written_files +from ._imported import Imported, as_imported, grafted, read_imported, source_of from ._secrets import credentials_in from .installed import installed_bundles from .ledger import Check, Ledger @@ -84,22 +86,27 @@ def register_bundle( It turns namespace routing on while it runs. Raises BundleError listing every problem found before anything is written; a write that fails stops it, and every earlier one stays, in the ledger, for the next run to reuse.""" + say = on_progress or (lambda line: None) with namespace_routing(): bundle = parse_bundle(Path(root)) + imported, why = read_imported(bundle.root) + if why is not None: + say(f" {why}") + bundle, grafts = grafted(bundle, imported) if imported is not None else (bundle, {}) resolved = resolve_bundle(bundle) plan = _plan(resolved, tasks, evals) - ids, problems = registry_ids(plan, _prefix(bundle, prefix)) + ids, problems = registry_ids(plan, _prefix(bundle, prefix), imported) if problems: raise BundleError(problems) registered = translated(plan, ids) target = configured_store(get_config().get_document_store()) shared = shared_store(target) identity, commit, dirty = bundle_identity(bundle.root) - ledger = RegistrationLedger(target, registered, identity, commit, dirty, frozenset(adopt)) - problems = [*_unowned(registered, ledger), *_unregistrable(registered, shared)] + sources, kept = _sources(plan, ids, imported) + ledger = RegistrationLedger(target, registered, identity, commit, dirty, frozenset(adopt), sources, kept) + problems = [*_unowned(registered, ledger), *_unregistrable(registered, shared, grafts)] if problems: raise BundleError(problems) - say = on_progress or (lambda line: None) materialization = materialize( registered, dry_run=dry_run, registering=Registering(ledger), on_wait=lambda: say("waiting for another agent-env run to finish writing this bundle's ids"), @@ -143,18 +150,24 @@ def _prefix(bundle: Bundle, prefix: str | None) -> str: return f"{name}-" -def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: +def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None) -> tuple[dict[str, str], list[str]]: """The registry id each ``@local`` id the plan writes takes, and each problem giving them. - The bundle's own entry is ````, its name lowercased with each run of what an id can't hold made + What ``imported`` records the folder was imported from takes the id it had there, and an imported task's outputs + the names its file gives them, as the stored task did. Any other entry of the bundle's own is ````, its name lowercased with each run of what an id can't hold made ``-``. An installed bundle's agent pulled in by a reference takes the first alias its bundle gives it, which the reference named it by; any other entity of an installed bundle has no registry id, and an env's alias can't be one, since no store env may start on an installed env alias's id. An id derived from another (``__env_image``) keeps its suffix after the other's registry id, and a task's output is ``-``.""" ids: dict[str, str] = {} problems: list[str] = [] + imported_tasks = set() for write in plan.writes: entry = write.source.entry + if imported is not None and (source := source_of(imported, plan, write)) is not None: + ids[write.id] = source[0] + imported_tasks.update([entry.id] if write.kind is BundleKind.TASK else []) + continue if entry.id in ids or (isinstance(write.source, BundleImage) and write.id != entry.id): continue if entry in plan.origins: @@ -174,12 +187,15 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: source = write.source if isinstance(source, ResolvedEntry) and source.entry.id in ids: for output in source.outputs: - name = _slug(output.id.removeprefix(f"{source.entry.id}/").rsplit("/", 1)[-1]) - ids[output.id] = f"{ids[source.entry.id]}-{name}" + written = output.id.removeprefix(f"{source.entry.id}/") + ids[output.id] = (written if source.entry.id in imported_tasks + else f"{ids[source.entry.id]}-{_slug(written.rsplit('/', 1)[-1])}") by_target: dict[str, list[str]] = {} + kept = {registry_id for local, registry_id in ids.items() + if imported is not None and local not in _given(ids, imported_tasks, plan, imported)} for local, registry_id in ids.items(): by_target.setdefault(registry_id, []).append(local) - if not _REGISTRY_ID.fullmatch(registry_id.replace("__", "_")): + if registry_id not in kept and not _REGISTRY_ID.fullmatch(registry_id.replace("__", "_")): problems.append(f"{registry_id!r}: isn't an id a registry takes; rename what it's made from") for registry_id, locals_ in by_target.items(): if len(locals_) > 1: @@ -187,6 +203,33 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: return ids, problems +def _given(ids: Mapping[str, str], imported_tasks: set[str], plan: Plan, imported: Imported) -> set[str]: + """The ``@local`` ids in ``ids`` whose registry ids the rules made, not ``imported``'s record: what isn't an + imported write, an imported task's output, or derived from either.""" + taken = {write.id for write in plan.writes if source_of(imported, plan, write) is not None} + taken |= {local for local in ids if local.rpartition("/")[0] in imported_tasks} + return {local for local in ids + if local not in taken and not any(local.startswith(f"{base}__") for base in taken)} + + +def _sources(plan: Plan, ids: Mapping[str, str], + imported: Imported | None) -> tuple[dict[tuple[str, str], int], set[tuple[str, str]]]: + """For each write of ``plan``, under its ``@local`` ids, that the folder was imported from, by (store, registry id), + the version it was imported at; and which of them the folder still holds as the import wrote it.""" + if imported is None: + return {}, set() + now = written_files(plan.bundle.bundle.root) + sources, kept = {}, set() + for write in plan.writes: + if (source := source_of(imported, plan, write)) is None: + continue + key = (write.kind.store, ids.get(write.id, write.id)) + sources[key] = source[1] + if as_imported(imported, plan, write, now): + kept.add(key) + return sources, kept + + def _alias(bundle: Bundle, entry: BundleEntry) -> str | None: aliases = bundle.aliases.get(AGENT_ALIASES, {}).get(entry.name, ()) if entry.kind is BundleKind.AGENT else () return aliases[0] if aliases else None @@ -278,13 +321,19 @@ class RegistrationLedger(Ledger): strict = True def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None, - adopt: frozenset[str] = frozenset()): + adopt: frozenset[str] = frozenset(), sources: Mapping[tuple[str, str], int] | None = None, + kept: set[tuple[str, str]] | None = None): super().__init__(store, plan) self.identity, self.commit, self.dirty, self.adopted_ids = identity, commit, dirty, adopt + self.sources, self.kept = sources or {}, kept or set() def check(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Check: """``Ledger.check``, refusing a write whose id the store holds that this bundle didn't register, checked again - as it's about to be written, since another bundle can write the id after the check before any write.""" + as it's about to be written, since another bundle can write the id after the check before any write. A write the + folder holds as its import wrote it, all it needs kept at the versions they were imported at, keeps the version + it was imported at, and the ledger records it.""" + if (kept := self._as_imported(write, needs)) is not None: + return kept check = super().check(write, needs) if (problem := self.unowned(write, check.stored)) is not None: raise BundleError([problem]) @@ -292,24 +341,46 @@ def check(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Check: def unowned(self, write: Write, stored: int | None) -> str | None: """Why this bundle may not write ``write``'s id, which the store holds at ``stored``, or None when it may: the - store doesn't hold it, ``adopt`` names it, this bundle registered its latest version, or an interrupted - registration of this bundle wrote it.""" + store doesn't hold it, ``adopt`` names it, an interrupted registration of this bundle wrote its latest version, + or that's the version this bundle last registered or its folder was imported from. One the store has written + anew since is refused, so no one else's version is written over unseen.""" if stored is None or write.id in self.adopted_ids: return None - row = self._latest(write.kind.store, write.id) - if row is not None and row.get("bundle") == self._bundle(write): - return None pending = self._find(self.collection, Filter.of(store=write.kind.store, id=write.id, bundle=self._bundle(write), status="pending")) if pending is not None and pending.get("version") == stored: return None + where, key = self._plan.path(write.source.entry), (write.kind.store, write.id) + row = self._latest(write.kind.store, write.id) + if row is not None and row.get("bundle") == self._bundle(write): + return None if row.get("version") == stored else self._moved(where, write.id, stored, row["version"], + "this bundle last registered") + if key in self.sources: + return None if stored == self.sources[key] else self._moved(where, write.id, stored, self.sources[key], + "this folder was imported from") by = f"registered from {row['bundle']}" if row is not None else "not registered from a bundle" - return (f"{self._plan.path(write.source.entry)}: {write.id!r} is in the store already, at v{stored}, {by}; pass " - f"--adopt {write.id} to write a new version of it, or --prefix to register this bundle under other ids") + return (f"{where}: {write.id!r} is in the store already, at v{stored}, {by}; pass --adopt {write.id} to write a " + "new version of it, or --prefix to register this bundle under other ids") + + @staticmethod + def _moved(where: str, entity_id: str, stored: int, own: int, whose: str) -> str: + return (f"{where}: {entity_id!r} is at v{stored} in the store, newer than the v{own} {whose}; pass --adopt " + f"{entity_id} to write over it") def stored_version(self, write: Write) -> int | None: return self._stored_version(write) + def _as_imported(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Check | None: + key = (write.kind.store, write.id) + if key not in self.kept or any(needs[need] != self.sources.get(need) for need in write.needs): + return None + stored = self._stored_version(write) + if stored is None or stored < self.sources[key]: + return None + digest = self.digest(write, needs) + return Check(write, digest, self.sources[key], ("as it was imported",), stored, needs, + adopted=digest is not None) + def _bundle(self, write: Write) -> str: """The bundle's identity, or for an entity an installed bundle comes with, that bundle's, which every bundle registering it shares.""" @@ -355,26 +426,34 @@ def _unowned(plan: Plan, ledger: RegistrationLedger) -> Iterator[str]: """A problem for each write this bundle may not make (``RegistrationLedger.unowned``), every one found before any write.""" for write in plan.writes: - if (problem := ledger.unowned(write, ledger.stored_version(write))) is not None: + kept = (write.kind.store, write.id) in ledger.kept # it may write nothing; its check decides + if not kept and (problem := ledger.unowned(write, ledger.stored_version(write))) is not None: yield problem -def _unregistrable(plan: Plan, shared: bool) -> Iterator[str]: +def _unregistrable(plan: Plan, shared: bool, grafts: Mapping[Path, set[str]]) -> Iterator[str]: """A problem for each ``@local`` id left in what the plan writes and, for a ``shared`` store, each credential - written out and each URL that reaches only this machine.""" + written out and each URL that reaches only this machine, but in a field given back from the stored task the folder + was imported from (``grafts``), which the store holds already.""" for write in plan.writes: if not isinstance(write.source, ResolvedEntry): continue where = plan.path(write.source.entry) + taken = grafts.get(write.source.entry.path, set()) + + def stored(path: str) -> bool: + return any(path == field or path.startswith((f"{field}.", f"{field}[")) for field in taken) + for path, text in _strings(write.source.config): if is_local_id(text): yield f"{where}: {path or 'its value'} names {text!r}, which only this machine's stores hold" - elif shared and (url := _local_url(text)) is not None: + elif shared and not stored(path) and (url := _local_url(text)) is not None: yield f"{where}: {path or 'its value'} names {url}, which only this machine reaches" if shared: for path, why in credentials_in(write.source.config): - yield (f"{where}: {path}: {why}, and a shared store shows it to anyone who reads it; leave it out, for " - "whatever runs the task to give it") + if not stored(path): + yield (f"{where}: {path}: {why}, and a shared store shows it to anyone who reads it; leave it out, " + "for whatever runs the task to give it") def _strings(value: Any, path: str = "") -> Iterator[tuple[str, str]]: diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py new file mode 100644 index 000000000..332f53098 --- /dev/null +++ b/tst/unit/bundle/register_imports_test.py @@ -0,0 +1,159 @@ +"""Registering a folder ``bundle init`` wrote back into the store it came from: each entity under the id it was +imported from, nothing written while the folder holds it as imported, an edit re-versioning only what it reaches, and +the user-scoped fields the import left out given back from the stored task.""" + +import json + +import pytest + +from agent_env.a2a_agent import A2AAgent +from agent_env.artifact import Artifact, FileArtifact +from agent_env.bundle import BundleError +from agent_env.bundle import _imported as imported_module +from agent_env.bundle import materialize as materialize_module +from agent_env.bundle import register as register_module +from agent_env.bundle.importer import import_task +from agent_env.bundle.register import REGISTRATIONS, register_bundle +from agent_env.bundle.resolve import build_step +from agent_env.config import get_config +from agent_env.env.envs.mcp_server import MCPServerEnv +from agent_env.store import Filter +from agent_env.task import Task +from tst.unit.bundle._support import DIGEST, context_image + +STEPS = [ + {"id": "env", "type": "deploy_env", "env_id": "crm-env"}, + {"id": "load", "type": "load_artifact", "env_id": "crm-env", "artifacts": [{"id": "crm-data"}]}, + {"id": "agent", "type": "deploy_agent", "env_ids": ["crm-env"], "a2a_agent_id": "solver", + "env_vars": {"API_KEY": "abc", "MODE": "fast"}}, +] + + +@pytest.fixture(autouse=True) +def outside_git(monkeypatch): + monkeypatch.setattr(register_module, "_git", lambda root, *args: None) + + +@pytest.fixture +def imported(local_stores, tmp_path, monkeypatch): + """The store's task triage, which names crm-env v1, crm-data v1 and the agent solver, imported into a folder.""" + monkeypatch.setenv("HOME", str(tmp_path)) + MCPServerEnv.put(id="crm-env", docker_image_artifact=context_image(tmp_path, "crm-image"), environment_name="crm") + A2AAgent.put(id="solver", docker_image_artifact=context_image(tmp_path, "solver-image")) + (tmp_path / "crm.json").write_text("{}") + FileArtifact.put("crm-data", description="crm seed", file_path=str(tmp_path / "crm.json")) + Task.put(id="triage", steps=[build_step(dict(step)) for step in STEPS]) + return import_task("triage", tmp_path / "triage").root + + +def _counts(): + documents = get_config().get_document_store() + return {name: documents.count(name, Filter()) for name in ("tasks", "envs", "a2a_agents", "artifacts")} + + +def _written(registration): + return {done.write.id: (done.version, done.reused) for done in registration.materialization.writes} + + +def test_a_folder_registered_as_it_was_imported_writes_nothing_and_keeps_each_version(imported): + before = _counts() + + registered = register_bundle(imported) + + assert _written(registered) == {"crm-image": (1, True), "crm-env": (1, True), "crm-data": (1, True), + "triage": (1, True)} + assert _counts() == before + rows = get_config().get_document_store().query(REGISTRATIONS, Filter.of(status="done")) + assert {(row["id"], row["version"]) for row in rows} == {("crm-image", 1), ("crm-env", 1), ("crm-data", 1), + ("triage", 1)} + assert _written(register_bundle(imported)) == _written(registered) + + +def test_an_edit_rewrites_only_what_it_reaches_and_gives_back_what_the_import_left_out(imported): + (imported / "artifacts/crm-data/crm.json").write_text('{"edited": true}') + + registered = register_bundle(imported) + + assert _written(registered) == {"crm-image": (1, True), "crm-env": (1, True), "crm-data": (2, False), + "triage": (2, False)} + steps = Task.get("triage", 2).to_dict()["steps"] + assert (steps[0]["env_id"], steps[0]["env_version"]) == ("crm-env", 1) + assert steps[1]["artifacts"] == [{"id": "crm-data", "version": 2}] + assert (steps[2]["a2a_agent_id"], steps[2]["a2a_agent_version"]) == ("solver", 1) + assert steps[2]["env_vars"] == {"API_KEY": "abc", "MODE": "fast"} + assert Artifact.get("crm-data", 2).description == "crm seed" + + +def test_a_given_back_field_the_store_holds_is_no_credential_written_out_to_a_shared_store(imported, monkeypatch): + monkeypatch.setattr(register_module, "shared_store", lambda store: True) + (imported / "artifacts/crm-data/crm.json").write_text('{"edited": true}') + + registered = register_bundle(imported) + + assert _written(registered)["triage"] == (2, False) + + +def test_an_edit_to_what_the_store_has_since_written_anew_is_refused_unless_adopted(imported, tmp_path): + FileArtifact.put("crm-data", description="someone else's", file_path=str(tmp_path / "crm.json")) + kept = register_bundle(imported) + (imported / "artifacts/crm-data/crm.json").write_text('{"edited": true}') + + with pytest.raises(BundleError) as refused: + register_bundle(imported) + adopted = register_bundle(imported, adopt=["crm-data"]) + + assert _written(kept)["crm-data"] == (1, True) + assert refused.value.problems == ("artifacts/crm-data: 'crm-data' is at v2 in the store, newer than the v1 this " + "bundle last registered; pass --adopt crm-data to write over it",) + assert _written(adopted)["crm-data"] == (3, False) + + +def test_an_image_whose_build_context_is_edited_is_pushed_and_its_env_rewritten(imported, monkeypatch): + monkeypatch.setattr(materialize_module, "push_context_image", + lambda entity_id, context, dockerfile, before=None: (f"registry.example.com/{entity_id}:x", + DIGEST)) + dockerfile = imported / "envs/crm-env/Dockerfile" + dockerfile.write_text(dockerfile.read_text() + "ENV EDITED=1\n") + + registered = register_bundle(imported) + + assert _written(registered) == {"crm-image": (2, False), "crm-env": (2, False), "crm-data": (1, True), + "triage": (2, False)} + + +def test_a_folder_imported_from_another_store_registers_as_a_new_bundle(imported, monkeypatch): + monkeypatch.setattr(imported_module, "store_fingerprint", lambda: "sha256:another-store") + said = [] + + registered = register_bundle(imported, dry_run=True, on_progress=said.append) + + assert " it was imported from another store, so its entries take new ids" in said + assert set(_written(registered)) == {"triage-crm-env__env_image", "triage-crm-env", "triage-crm-data", + "triage-triage"} + + +def test_a_record_from_an_older_agent_env_registers_the_folder_as_a_new_bundle(imported): + record = json.loads((imported / "provenance.json").read_text()) + (imported / "provenance.json").write_text(json.dumps({**record, "schema": 1})) + said = [] + + registered = register_bundle(imported, dry_run=True, on_progress=said.append) + + assert " provenance.json is from an older agent-env, so the folder registers as a new bundle" in said + assert set(_written(registered)) == {"triage-crm-env__env_image", "triage-crm-env", "triage-crm-data", + "triage-triage"} + + +def test_an_imported_task_with_an_output_keeps_the_output_s_name(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + MCPServerEnv.put(id="crm-env", docker_image_artifact=context_image(tmp_path, "crm-image"), environment_name="crm") + Task.put(id="cli", steps=[build_step({"id": "env", "type": "deploy_env", "env_id": "crm-env"}), + build_step({"id": "cli", "type": "build_mcp_cli", "env_id": "crm-env", + "command_name": "crm", "cli_artifact_id": "crm-cli"})]) + folder = import_task("cli", tmp_path / "cli").root + (folder / "tasks/cli.json").write_text((folder / "tasks/cli.json").read_text().replace('"crm"', '"crm2"')) + + registered = register_bundle(folder) + + assert Task.get("cli", 2).to_dict()["steps"][1]["cli_artifact_id"] == "crm-cli" + assert _written(registered)["cli"] == (2, False) diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 6ca71b66a..37002c5f7 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -518,3 +518,19 @@ def then_another_bundle_writes(plan, ledger): assert "'triage-docs' is in the store already, at v1, registered from folder:/elsewhere/triage" in \ refused.value.problems[0] assert Artifact.get("triage-docs").description == "theirs" + + +def test_a_version_someone_else_wrote_since_is_not_written_over_unless_adopted(home): + root = layout(home / "triage", TRIAGE) + register_bundle(root) + (home / "seed.txt").write_text("seed") + FileArtifact.put("triage-docs", description="someone else's", file_path=str(home / "seed.txt")) + (root / "artifacts/docs/a.md").write_text("changed\n") + + with pytest.raises(BundleError) as refused: + register_bundle(root) + adopted = register_bundle(root, adopt=["triage-docs"]) + + assert refused.value.problems == ("artifacts/docs: 'triage-docs' is at v2 in the store, newer than the v1 this " + "bundle last registered; pass --adopt triage-docs to write over it",) + assert {done.write.id: done.version for done in adopted.materialization.writes}["triage-docs"] == 3 From d469357ab4314a8f5bc6177f3b78b809ca0bff3a Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 13:38:39 -0700 Subject: [PATCH 07/22] fix(bundle): don't register over a version someone else wrote since The bundle's own ledger row let it write an id whatever the store's latest was, so a version another tool or person wrote after the last registration was written over unseen. Now an id whose latest version isn't the one this bundle last registered is refused unless --adopt. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/register.py | 22 +++++++++++++++------- tst/unit/bundle/register_test.py | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index b2838d6e0..ed139d72a 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -292,20 +292,28 @@ def check(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Check: def unowned(self, write: Write, stored: int | None) -> str | None: """Why this bundle may not write ``write``'s id, which the store holds at ``stored``, or None when it may: the - store doesn't hold it, ``adopt`` names it, this bundle registered its latest version, or an interrupted - registration of this bundle wrote it.""" + store doesn't hold it, ``adopt`` names it, an interrupted registration of this bundle wrote its latest version, + or that's the version this bundle last registered. One the store has written anew since is refused, so no one + else's version is written over unseen.""" if stored is None or write.id in self.adopted_ids: return None - row = self._latest(write.kind.store, write.id) - if row is not None and row.get("bundle") == self._bundle(write): - return None pending = self._find(self.collection, Filter.of(store=write.kind.store, id=write.id, bundle=self._bundle(write), status="pending")) if pending is not None and pending.get("version") == stored: return None + where = self._plan.path(write.source.entry) + row = self._latest(write.kind.store, write.id) + if row is not None and row.get("bundle") == self._bundle(write): + return None if row.get("version") == stored else self._moved(where, write.id, stored, row["version"], + "this bundle last registered") by = f"registered from {row['bundle']}" if row is not None else "not registered from a bundle" - return (f"{self._plan.path(write.source.entry)}: {write.id!r} is in the store already, at v{stored}, {by}; pass " - f"--adopt {write.id} to write a new version of it, or --prefix to register this bundle under other ids") + return (f"{where}: {write.id!r} is in the store already, at v{stored}, {by}; pass --adopt {write.id} to write a " + "new version of it, or --prefix to register this bundle under other ids") + + @staticmethod + def _moved(where: str, entity_id: str, stored: int, own: int, whose: str) -> str: + return (f"{where}: {entity_id!r} is at v{stored} in the store, newer than the v{own} {whose}; pass --adopt " + f"{entity_id} to write over it") def stored_version(self, write: Write) -> int | None: return self._stored_version(write) diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 6ca71b66a..37002c5f7 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -518,3 +518,19 @@ def then_another_bundle_writes(plan, ledger): assert "'triage-docs' is in the store already, at v1, registered from folder:/elsewhere/triage" in \ refused.value.problems[0] assert Artifact.get("triage-docs").description == "theirs" + + +def test_a_version_someone_else_wrote_since_is_not_written_over_unless_adopted(home): + root = layout(home / "triage", TRIAGE) + register_bundle(root) + (home / "seed.txt").write_text("seed") + FileArtifact.put("triage-docs", description="someone else's", file_path=str(home / "seed.txt")) + (root / "artifacts/docs/a.md").write_text("changed\n") + + with pytest.raises(BundleError) as refused: + register_bundle(root) + adopted = register_bundle(root, adopt=["triage-docs"]) + + assert refused.value.problems == ("artifacts/docs: 'triage-docs' is at v2 in the store, newer than the v1 this " + "bundle last registered; pass --adopt triage-docs to write over it",) + assert {done.write.id: done.version for done in adopted.materialization.writes}["triage-docs"] == 3 From d89086f1343c516553183a9f70ec8d7869f728cc Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 15:04:07 -0700 Subject: [PATCH 08/22] fix(bundle): refuse an env:/secret: reference in a step's env vars, which nothing resolves The shared-store check let a secret-named env var through when its value was a secret: reference, as if the run would resolve it. Nothing resolves env: or secret: references in a step's env vars, so the run would get the reference itself. Such a value is now refused, whatever the variable's name, with a reason saying so. The other refusals now say what each one exposes. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_secrets.py | 22 ++++++++++++++-------- src/agent_env/bundle/register.py | 3 +-- tst/unit/bundle/register_test.py | 13 +++++++++---- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/agent_env/bundle/_secrets.py b/src/agent_env/bundle/_secrets.py index 1a5a279cf..c79262ea8 100644 --- a/src/agent_env/bundle/_secrets.py +++ b/src/agent_env/bundle/_secrets.py @@ -6,7 +6,7 @@ from typing import Any from agent_env.config.describe import is_secret_key -from agent_env.config.loader import SECRET_REF_PREFIX +from agent_env.config.loader import ENV_REF_PREFIX, SECRET_REF_PREFIX # The fields that map environment variable names to values, where a credential is most often set. _ENV_VAR_MAPS = ("env_vars", "default_env_vars") @@ -62,23 +62,29 @@ def _masked(text: str) -> tuple[str, list[str]]: def credentials_in(value: Any, path: str = "") -> list[tuple[str, str]]: - """Each credential ``value`` holds written out, by its path, and what it is: an env var whose name looks like a - secret's, set to anything but a ``secret:`` reference, and any value holding a credential ``scrub`` masks.""" + """Each secret ``value`` holds that a store would keep as written, by its path, and why: an env var whose name looks + like a secret's set to anything, any value holding a credential ``scrub`` masks, and an env var set to an ``env:`` + or ``secret:`` reference, which nothing resolves in a step's env vars, so the run would get the reference itself.""" found = [] items = value.items() if isinstance(value, dict) else enumerate(value) if isinstance(value, list) else () for key, item in items: at = f"{path}.{key}" if path and isinstance(key, str) else f"{path}[{key}]" if isinstance(key, int) else key if isinstance(value, dict) and key in _ENV_VAR_MAPS and isinstance(item, dict): for name, setting in item.items(): - if not isinstance(setting, str) or not setting or setting.startswith(SECRET_REF_PREFIX): + if not isinstance(setting, str) or not setting: continue - if is_secret_key(name): - found.append((f"{at}.{name}", "its name looks like a secret's, and it's set")) + if setting.startswith((ENV_REF_PREFIX, SECRET_REF_PREFIX)): + found.append((f"{at}.{name}", "it's a reference, and nothing resolves one in a step's env vars, so " + "the run would get the reference itself")) + elif is_secret_key(name): + found.append((f"{at}.{name}", "its name looks like a secret's and it's set, which a shared store " + "shows to anyone who reads it")) elif kinds := _masked(setting)[1]: - found.append((f"{at}.{name}", f"it holds {kinds[0]}")) + found.append((f"{at}.{name}", f"it holds {kinds[0]}, which a shared store shows to anyone who " + "reads it")) elif isinstance(item, str): if kinds := _masked(item)[1]: - found.append((at, f"it holds {', '.join(kinds)}")) + found.append((at, f"it holds {', '.join(kinds)}, which a shared store shows to anyone who reads it")) else: found += credentials_in(item, at) return found diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index ed139d72a..573151931 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -381,8 +381,7 @@ def _unregistrable(plan: Plan, shared: bool) -> Iterator[str]: yield f"{where}: {path or 'its value'} names {url}, which only this machine reaches" if shared: for path, why in credentials_in(write.source.config): - yield (f"{where}: {path}: {why}, and a shared store shows it to anyone who reads it; leave it out, for " - "whatever runs the task to give it") + yield f"{where}: {path}: {why}; leave it out, for whatever runs the task to give it" def _strings(value: Any, path: str = "") -> Iterator[tuple[str, str]]: diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 37002c5f7..9d51a0168 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -298,18 +298,23 @@ def test_an_at_local_id_left_in_what_would_be_written_is_refused(home): "tasks/t.json: [0].metadata.from names '@local/~/elsewhere/x', which only this machine's stores hold",) -def test_a_shared_store_refuses_a_credential_written_out_and_a_url_only_this_machine_reaches(home, monkeypatch): +def test_a_shared_store_refuses_a_credential_or_reference_in_env_vars_and_a_url_only_this_machine_reaches( + home, monkeypatch): monkeypatch.setattr(register_module, "shared_store", lambda store: True) files = {"tasks/t.json": json.dumps([ - {**BOX, "env_vars": {"API_KEY": "abc", "TOKEN": "secret:team_token", "MODE": "fast"}}, + {**BOX, "env_vars": {"API_KEY": "abc", "TOKEN": "secret:team_token", "MODE": "fast", "DB_URL": "env:DB_URL"}}, {"id": "agent", "type": "deploy_agent", "env_ids": [], "metadata": {"wiki": "http://localhost:8080/x"}}])} with pytest.raises(BundleError) as refused: register_bundle(layout(home / "triage", files)) assert refused.value.problems == ( - "tasks/t.json: [0].env_vars.API_KEY: its name looks like a secret's, and it's set, and a shared store shows " - "it to anyone who reads it; leave it out, for whatever runs the task to give it", + "tasks/t.json: [0].env_vars.API_KEY: its name looks like a secret's and it's set, which a shared store shows " + "to anyone who reads it; leave it out, for whatever runs the task to give it", + "tasks/t.json: [0].env_vars.DB_URL: it's a reference, and nothing resolves one in a step's env vars, so the run " + "would get the reference itself; leave it out, for whatever runs the task to give it", + "tasks/t.json: [0].env_vars.TOKEN: it's a reference, and nothing resolves one in a step's env vars, so the run " + "would get the reference itself; leave it out, for whatever runs the task to give it", "tasks/t.json: [1].metadata.wiki names http://localhost:8080, which only this machine reaches") From 0141fbef12b7dc2180845c2392c30c5db6426a6f Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 15:06:40 -0700 Subject: [PATCH 09/22] test(bundle): join the remote URLs' fake credentials in at run time The tests that a bundle's identity drops a remote's credentials held URLs with user info written out, which the pre-commit secret scan flags as new secrets. Co-Authored-By: Claude Opus 5.5 --- tst/unit/bundle/register_test.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 9d51a0168..dd46e2972 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -32,6 +32,8 @@ from tst.unit.bundle._support import DIGEST, layout BOX = {"id": "box", "type": "deploy_sandbox", "sandbox_name": "box", "sandbox_mode": "vm"} +# Not a credential: what a remote URL's user info holds in the tests that its name drops, joined in at run time. +_FAKE_TOKEN = "not-a-token" def _load(step_id, artifact): @@ -134,7 +136,8 @@ def test_the_ledger_rows_are_in_the_target_store_and_name_the_bundle_by_its_remo monkeypatch.setattr(register_module, "_git", _GIT) repo = tmp_path / "repo" layout(repo / "bundles" / "triage", TRIAGE) - for args in (["init", "-q"], ["remote", "add", "origin", "https://user:token123@github.com/team/repo.git"], + remote = "https://user:" + _FAKE_TOKEN + "@github.com/team/repo.git" + for args in (["init", "-q"], ["remote", "add", "origin", remote], ["add", "."], ["-c", "user.email=a@b.c", "-c", "user.name=a", "commit", "-q", "-m", "x"]): subprocess.run(["git", "-C", str(repo), *args], check=True) @@ -145,12 +148,12 @@ def test_the_ledger_rows_are_in_the_target_store_and_name_the_bundle_by_its_remo assert {row["id"] for row in rows} == {"triage-docs", "triage-greeting", "triage-t"} assert {row["bundle"] for row in rows} == {"github.com/team/repo:bundles/triage"} assert all(row["commit"] and row["dirty"] is False for row in rows) - assert "token123" not in json.dumps(rows, default=str) + assert _FAKE_TOKEN not in json.dumps(rows, default=str) @pytest.mark.parametrize(("remote", "name"), [ ("git@github.com:team/repo.git", "github.com/team/repo"), - ("https://x-access-token:ghs_abc@github.com/team/repo", "github.com/team/repo"), + ("https://x-access-token:" + _FAKE_TOKEN + "@github.com/team/repo", "github.com/team/repo"), ("ssh://git@gitlab.example.com:2222/group/sub/repo.git", "gitlab.example.com/group/sub/repo"), ]) def test_a_remote_is_named_by_its_host_and_path_alone(remote, name): From f69989cdc74e1c8bcba66f0232a447f6cbf6dc5c Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 15:58:52 -0700 Subject: [PATCH 10/22] fix(bundle): check duplicate registry ids per store, and name the git timeout A task and an artifact whose names make one id live in different stores, so they no longer refuse each other. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/register.py | 30 ++++++++++++++++++++---------- tst/unit/bundle/register_test.py | 9 +++++++++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 573151931..959671d1b 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -8,12 +8,13 @@ changes meaning. Each image the bundle builds is built here for linux/amd64 and pushed to the configured image store under a tag of its sources, with its build context beside it; one published for its sources is registered as published. -Nothing is written when any problem is found before the first write: an id the rules can't give, two writes given one -id, an id the store holds that this bundle didn't register (unless adopted), an ``@local`` id left in what would be -written, a store reference the store doesn't hold, or, for a store that isn't this machine's, a credential written out -or a URL that only reaches this machine. A dry run checks the same and writes, builds and pushes nothing. Each write -goes through a ledger kept in the target store, so registering again writes only what changed. Two registrations -racing for one id can't both write a version of it: the loser fails, and running it again reuses what the winner wrote. +Nothing is written when any problem is found before the first write: an id the rules can't give, two writes to one store +given one id, an id the store holds that this bundle didn't register (unless adopted), an ``@local`` id left in what +would be written, a store reference the store doesn't hold, or, for a store that isn't this machine's, a credential +written out or a URL that only reaches this machine. A dry run checks the same and writes, builds and pushes nothing. +Each write goes through a ledger kept in the target store, so registering again writes only what changed. Two +registrations racing for one id can't both write a version of it: the loser fails, and running it again reuses what the +winner wrote. """ from __future__ import annotations @@ -52,6 +53,7 @@ _REGISTRY_ID = re.compile(r"[a-z0-9][a-z0-9._-]*") _NOT_IN_ID = re.compile(r"[^a-z0-9._-]+") _URL = re.compile(r"\b(?:file:///?[^\s\"']*|[a-z][a-z0-9+.-]*://[^\s/\"']+)", re.IGNORECASE) +_GIT_TIMEOUT_SECONDS = 10 @dataclass(frozen=True) @@ -176,12 +178,19 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: for output in source.outputs: name = _slug(output.id.removeprefix(f"{source.entry.id}/").rsplit("/", 1)[-1]) ids[output.id] = f"{ids[source.entry.id]}-{name}" - by_target: dict[str, list[str]] = {} + stores: dict[str, set[str]] = {} + for write in plan.writes: + stores.setdefault(write.id, set()).add(write.kind.store) + if isinstance(write.source, ResolvedEntry): + for output in write.source.outputs: + stores.setdefault(output.id, set()).add(BundleKind.ARTIFACT.store) + by_target: dict[tuple[str, str], list[str]] = {} for local, registry_id in ids.items(): - by_target.setdefault(registry_id, []).append(local) + for store in stores.get(local, ()): + by_target.setdefault((store, registry_id), []).append(local) if not _REGISTRY_ID.fullmatch(registry_id.replace("__", "_")): problems.append(f"{registry_id!r}: isn't an id a registry takes; rename what it's made from") - for registry_id, locals_ in by_target.items(): + for (_, registry_id), locals_ in by_target.items(): if len(locals_) > 1: problems.append(f"{', '.join(sorted(locals_))} would all be registered as {registry_id!r}; rename one") return ids, problems @@ -343,7 +352,8 @@ def bundle_identity(root: Path) -> tuple[str, str | None, bool | None]: def _git(root: Path, *args: str) -> str | None: try: - result = subprocess.run(["git", "-C", str(root), *args], capture_output=True, text=True, timeout=10) + result = subprocess.run(["git", "-C", str(root), *args], capture_output=True, text=True, + timeout=_GIT_TIMEOUT_SECONDS) except (OSError, subprocess.TimeoutExpired): return None return result.stdout.strip() if result.returncode == 0 else None diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index dd46e2972..305cb730a 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -202,6 +202,15 @@ def test_a_name_an_id_cant_hold_is_made_one_and_names_that_collide_are_refused(h "registered as 'big-triage-my-docs'; rename one",) +def test_entries_whose_names_make_one_id_register_in_their_own_stores(home): + files = {"artifacts/Echo/a.md": "a\n", "tasks/echo.json": json.dumps([BOX, _load("greet", "Echo")])} + + registered = register_bundle(layout(home / "triage", files)) + + assert {(done.write.kind.store, done.write.id) for done in registered.materialization.writes} == { + ("artifact", "triage-echo"), ("task", "triage-echo")} + + def test_a_bad_prefix_is_refused(home): with pytest.raises(BundleError) as refused: register_bundle(layout(home / "triage", TRIAGE), prefix="Team/") From 908c8d99faeb9c4edcb96e88f112217abbe5fc6e Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 16:12:33 -0700 Subject: [PATCH 11/22] fix(bundle): give back what an import left out, and keep each imported id its own, when registering the folder - A pin is given back only beside the reference it was stored with, not beside an entity the import wrote into the folder. - An env var the import dropped and a credential it masked come back where the folder still holds what it left. - A list item, such as a skill, takes back its left-out source from the stored item it was written from. - Envs sharing one stored image register as imported; an edit to one copy is refused. - An image whose toml names another Dockerfile is rebuilt. - An installed bundle's entries take nothing from the folder's record, and an output under a folder keeps its name. - The import numbers a name another kind holds for another id, and registering refuses one name taking two ids. - The store fingerprint resolves env: references and ignores credentials, and registering reads only the files of what the import wrote. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_import_report.py | 20 ++- src/agent_env/bundle/_imported.py | 147 +++++++++++++++++--- src/agent_env/bundle/importer.py | 9 +- src/agent_env/bundle/register.py | 46 ++++-- tst/unit/bundle/importer_entities_test.py | 22 ++- tst/unit/bundle/register_imports_test.py | 162 +++++++++++++++++++++- 6 files changed, 363 insertions(+), 43 deletions(-) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index 2d6fe263c..a096b7f82 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -7,13 +7,16 @@ import json import posixpath import unicodedata +from collections.abc import Iterable from dataclasses import dataclass from datetime import datetime, timezone from importlib.metadata import version as package_version from pathlib import Path from agent_env.bundle._fs import file_sha256 +from agent_env.bundle._secrets import scrub from agent_env.config import get_config +from agent_env.config.loader import SECRET_REF_PREFIX, interpolate from agent_env.config.model import ENV_API_KEY, ENV_BASE_URL from agent_env.entity.refs import EntityKind from agent_env.store.image_store.oci_registry_credentials import registry_host_from_ref @@ -185,18 +188,21 @@ def provenance(source: dict, imported: BundleImport, root: Path) -> str: def store_fingerprint() -> str: - """A hash of the configured document store's section, which tells one store from another without naming where - either is.""" - section = get_config().trace_section("document").value + """A hash of the configured document store's section, with its ``env:`` references resolved and any credential + in it masked, which tells one store from another without naming where either is.""" + section = interpolate(get_config().trace_section("document").value, + secret_resolver=lambda key: SECRET_REF_PREFIX + key) + scrub(section) text = json.dumps(section, sort_keys=True, separators=(",", ":"), default=str) return "sha256:" + hashlib.sha256(text.encode()).hexdigest() -def written_files(root: Path) -> dict[str, str]: - """Each file under ``root`` but its README and provenance, by its path in NFC, as its content hash, ``+x`` after it - when it's executable.""" +def written_files(root: Path, within: Iterable[str] | None = None) -> dict[str, str]: + """Each file under ``root``, or under its paths ``within``, but its README and provenance, by its path in NFC, as its + content hash, ``+x`` after it when it's executable.""" + tops = [root] if within is None else [root / part for part in within] files = {} - for path in sorted(root.rglob("*")): + for path in sorted({path for top in tops for path in ([top] if top.is_file() else top.rglob("*"))}): relative = unicodedata.normalize("NFC", path.relative_to(root).as_posix()) if path.is_file() and relative not in ("README.md", PROVENANCE): files[relative] = file_sha256(path) + ("+x" if path.stat().st_mode & 0o111 else "") diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index 7d59463e6..bd1d4a969 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -12,6 +12,7 @@ from pathlib import Path from agent_env.entity.paths import field_sites +from agent_env.entity.refs import ref_sites from agent_env.entity.scopes import FieldScope from agent_env.env.registry import get_env_registry from agent_env.store.base import NotFoundError @@ -19,6 +20,7 @@ from agent_env.task_step.registry import get_task_step_registry from ._import_report import PROVENANCE, PROVENANCE_SCHEMA, store_fingerprint +from ._secrets import scrub from .parse import CONFIG_FILES, Bundle, BundleEntry, BundleKind from .plan import Plan, Write, env_folder_writer from .resolve import BuiltImage, BundleImage @@ -27,11 +29,12 @@ @dataclass(frozen=True) class Imported: """What an import recorded: each entity folder, task file and eval file's store id and version, each image's by its - folder and toml key, and each file as it wrote it.""" + folder and toml key, with the Dockerfile it's built from, and each file as it wrote it.""" entities: Mapping[str, tuple[str, int]] images: Mapping[tuple[str, str], tuple[str, int]] files: Mapping[str, str] + dockerfiles: Mapping[tuple[str, str], str] def read_imported(root: Path) -> tuple[Imported | None, str | None]: @@ -53,7 +56,8 @@ def read_imported(root: Path) -> tuple[Imported | None, str | None]: if (stored_eval := record.get("eval")) is not None: entities[stored_eval["file"]] = (stored_eval["id"], stored_eval["version"]) images = {(item["folder"], item["key"]): (item["id"], item["version"]) for item in record.get("images", [])} - return Imported(entities, images, record.get("files", {})), None + dockerfiles = {(item["folder"], item["key"]): item.get("dockerfile") for item in record.get("images", [])} + return Imported(entities, images, record.get("files", {}), dockerfiles), None def where(root: Path, entry: BundleEntry) -> str: @@ -62,9 +66,12 @@ def where(root: Path, entry: BundleEntry) -> str: def source_of(imported: Imported, plan: Plan, write: Write) -> tuple[str, int] | None: - """The store id and version ``write`` was imported from, or None when the import didn't write it.""" + """The store id and version ``write`` was imported from, or None when the import didn't write it, as for what an + installed bundle holds.""" entry = write.source.entry - folder = where(plan.bundle_of(entry).root, entry) + if entry in plan.origins: + return None + folder = where(plan.bundle.bundle.root, entry) if isinstance(write.source, BundleImage) and write.source.id != entry.id: return imported.images.get((folder, image_key(write.source))) return imported.entities.get(folder) @@ -77,27 +84,32 @@ def image_key(image: BundleImage) -> str: def as_imported(imported: Imported, plan: Plan, write: Write, now: Mapping[str, str]) -> bool: - """Whether the files ``write`` is made from are those the import wrote: a task's or eval's file; an image's build - context, its folder but the toml; an env's or agent's toml, or every file of an env type written from its folder; - every file of an artifact or skill folder.""" + """Whether the files ``write`` is made from are those the import wrote: a task's or eval's file; an image a toml + key builds, its folder but the toml and the Dockerfile the key names; an env's or agent's toml, or every file of an + env type written from its folder; every file of an artifact, skill or image folder.""" entry = write.source.entry - folder = where(plan.bundle_of(entry).root, entry) + folder = where(plan.bundle.bundle.root, entry) if write.kind in (BundleKind.TASK, BundleKind.EVAL): return folder in imported.files and imported.files[folder] == now.get(folder) toml = f"{folder}/{CONFIG_FILES[entry.kind]}" if entry.kind in CONFIG_FILES else None then, current = _within(imported.files, folder), _within(now, folder) - if isinstance(write.source, BuiltImage): + if isinstance(write.source, BuiltImage) and write.source.id != entry.id: + if imported.dockerfiles.get((folder, image_key(write.source))) != write.source.dockerfile: + return False then.pop(toml, None) current.pop(toml, None) - elif isinstance(write.source, BundleImage) or (entry.kind in (BundleKind.ENV, BundleKind.AGENT) and not ( - entry.kind is BundleKind.ENV and _written_from_folder(entry))): + elif (isinstance(write.source, BundleImage) and write.source.id != entry.id) or ( + entry.kind in (BundleKind.ENV, BundleKind.AGENT) and not ( + entry.kind is BundleKind.ENV and _written_from_folder(entry))): then, current = {toml: then.get(toml)}, {toml: current.get(toml)} return then == current def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]]]: - """``bundle`` with each imported task's steps given back the user-scoped fields the import left out, from the - stored version it was imported from, matched by step id and type, where the folder doesn't set them; and, for each + """``bundle`` with each imported task's steps given back, from the stored version each was imported from, matched + by step id and type, what the import left out where the folder doesn't set it: each user-scoped field, but a + reference's pin where the folder names something else; each list item's, from the stored item it was written from; + and each env var and credential the import's scrub took, where the folder holds what the scrub left. And, for each task's file, the paths in its steps that took one.""" registry = get_task_step_registry() entries, taken = [], {} @@ -117,16 +129,113 @@ def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[ cls = registry.get(step.get("type")) if origin is None or cls is None or origin.get("type") != step.get("type"): continue - for path, scope in cls.field_scopes.items(): - if scope is not FieldScope.USER or "[]" in path: - continue - for _, holder, key in field_sites(origin, path): - if holder[key] not in ("", [], {}) and _graft(step, path.split("."), holder[key]): - taken.setdefault(entry.path, set()).add(f"[{index}].{path}") + for path in _given_back(step, origin, cls): + taken.setdefault(entry.path, set()).add(f"[{index}].{path}") entries.append(replace(entry, config=steps)) return replace(bundle, entries=tuple(entries)), taken +def _given_back(step: dict, origin: dict, cls: type) -> list[str]: + """Give ``step`` back what the import left out of ``origin``, in place; the path of each field it took.""" + user = [path for path, scope in cls.field_scopes.items() if scope is FieldScope.USER] + repinned = _repinned(step, origin, cls) + given = [] + for path in user: + if path in repinned: + continue + if "[]" in path: + given += _given_back_items(step, origin, path, user) + continue + for _, holder, key in field_sites(origin, path): + if holder[key] not in ("", [], {}) and _graft(step, path.split("."), holder[key]): + given.append(path) + scrubbed = copy.deepcopy(origin) + scrub(scrubbed) + return given + _unscrubbed(step, origin, scrubbed) + + +def _repinned(step: dict, origin: dict, cls: type) -> set[str]: + """The path of each pin in ``origin`` beside a reference ``step`` names something else at, such as an entity the + import wrote into the folder, which can't take the stored entity's pin.""" + refs = cls.entity_refs or () + held = {site.path: site.value for site in ref_sites(refs, step)} + return {site.path[:len(site.path) - len(str(site.key))] + site.version_key for site in ref_sites(refs, origin) + if site.version_key is not None and held.get(site.path, site.value) != site.value} + + +def _given_back_items(step: dict, origin: dict, path: str, user: list[str]) -> list[str]: + """Give each item of the list ``path`` names, ``[].``, its field from the stored item it was written + from: the one it equals but for the list's user-scoped fields, else the one alone in sharing its name.""" + at, _, field = path.partition("[].") + if "[]" in field: + return [] + items, stored = _at(step, at), _at(origin, at) + if not isinstance(items, list) or not isinstance(stored, list): + return [] + left_out = {other.partition("[].")[2].split(".")[0] for other in user if other.startswith(f"{at}[].")} + given = [] + for index, item in enumerate(items): + if (source := _source_item(item, stored, left_out)) is None: + continue + for _, holder, key in field_sites(source, field): + if holder[key] not in ("", [], {}) and _graft(item, field.split("."), holder[key]): + given.append(f"{at}[{index}].{field}") + return given + + +def _source_item(item: object, stored: list, left_out: set[str]) -> dict | None: + if not isinstance(item, dict): + return None + candidates = [one for one in stored if isinstance(one, dict)] + same = [one for one in candidates if _bare(one, left_out) == _bare(item, left_out)] + if not same and item.get("name") is not None: + same = [one for one in candidates if one.get("name") == item["name"]] + return same[0] if len(same) == 1 else None + + +def _bare(item: dict, left_out: set[str]) -> dict: + return {key: value for key, value in item.items() if key not in left_out and value is not None} + + +def _at(step: dict, path: str) -> object: + value = step + for key in path.split("."): + value = value.get(key) if isinstance(value, dict) else None + return value + + +def _unscrubbed(value: object, stored: object, scrubbed: object, path: str = "") -> list[str]: + """Put back in ``value``, in place, what scrubbing ``stored`` took where ``value`` holds what it left + (``scrubbed``): each env var it dropped that ``value`` doesn't set, and each credential it masked; each one's + path.""" + given = [] + if isinstance(value, dict) and isinstance(stored, dict) and isinstance(scrubbed, dict): + for key, item in stored.items(): + at = f"{path}.{key}" if path else str(key) + if key not in scrubbed: + if key not in value: + value[key] = copy.deepcopy(item) + given.append(at) + elif key in value: + if isinstance(item, str): + if value[key] == scrubbed[key] != item: + value[key] = item + given.append(at) + else: + given += _unscrubbed(value[key], item, scrubbed[key], at) + elif isinstance(value, list) and isinstance(stored, list) and isinstance(scrubbed, list) and ( + len(value) == len(stored) == len(scrubbed)): + for index, item in enumerate(stored): + at = f"{path}[{index}]" + if isinstance(item, str): + if value[index] == scrubbed[index] != item: + value[index] = item + given.append(at) + else: + given += _unscrubbed(value[index], item, scrubbed[index], at) + return given + + def _graft(step: dict, keys: list[str], value: object) -> bool: """Set ``keys`` in ``step`` to ``value`` when it's unset there; whether it was.""" holder = step diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index 9339ed962..a46c4f572 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -163,6 +163,7 @@ def __init__(self, dest: Path, include_scopes: Collection[FieldScope] = (), self.progress = progress or (lambda message: None) self.files: dict[str, str] = {} self.taken: dict[str, set[str]] = {kind.value: set() for kind in BundleKind} # folded names, per folder + self.named: dict[str, str] = {} # each folded name, and the id it was taken for self.tasks: list[ImportedTask] = [] self.eval: ImportedEval | None = None self.written: list[Written] = [] @@ -417,6 +418,7 @@ def _folder(self, kind: EntityKind, entity: Any) -> tuple[BundleKind, str]: raise Unwritable(f"a skill's folder takes its skill's name, {name!r}, which another artifact in the " "folder already has") self.taken[BundleKind.ARTIFACT.value].add(fold(name)) + self.named.setdefault(fold(name), entity.id) return BundleKind.SKILL, name bundle_kind = _FOLDERS[kind] return bundle_kind, self._name(bundle_kind.value, entity.id) @@ -500,14 +502,17 @@ def _default_agents(self, step: dict, refs: tuple[EntityRef, ...], where: str) - "default")) def _name(self, kind: str, entity_id: str) -> str: - """A name in the ``kind`` folder: the id's last segment made valid, numbered when taken, caselessly.""" + """A name in the ``kind`` folder: the id's last segment made valid, numbered when taken, caselessly, or when + another kind's folder holds it for another id, since entries of every kind that share a name share a local id, + and registering the folder gives each the id it was imported from.""" names = _names_of(BundleKind(kind)) base = _valid_name(entity_id.rsplit("/", 1)[-1]) or kind.removesuffix("s") name, n = base, 1 - while fold(name) in self.taken[names]: + while fold(name) in self.taken[names] or self.named.get(fold(name), entity_id) != entity_id: n += 1 name = f"{base}-{n}" self.taken[names].add(fold(name)) + self.named[fold(name)] = entity_id return name diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 53a560ded..765ac5282 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -40,7 +40,7 @@ from agent_env.task_step.registry import get_task_step_registry from ._import_report import written_files -from ._imported import Imported, as_imported, grafted, read_imported, source_of +from ._imported import Imported, as_imported, grafted, read_imported, source_of, where from ._secrets import credentials_in from .installed import installed_bundles from .ledger import Check, Ledger @@ -97,14 +97,15 @@ def register_bundle( bundle, grafts = grafted(bundle, imported) if imported is not None else (bundle, {}) resolved = resolve_bundle(bundle) plan = _plan(resolved, tasks, evals) - ids, problems = registry_ids(plan, _prefix(bundle, prefix), imported) + held = _held_as_imported(plan, imported) + ids, problems = registry_ids(plan, _prefix(bundle, prefix), imported, held) if problems: raise BundleError(problems) registered = translated(plan, ids) target = configured_store(get_config().get_document_store()) shared = shared_store(target) identity, commit, dirty = bundle_identity(bundle.root) - sources, kept = _sources(plan, ids, imported) + sources, kept = _sources(plan, ids, imported, held) ledger = RegistrationLedger(target, registered, identity, commit, dirty, frozenset(adopt), sources, kept) problems = [*_unowned(registered, ledger), *_unregistrable(registered, shared, grafts)] if problems: @@ -152,7 +153,8 @@ def _prefix(bundle: Bundle, prefix: str | None) -> str: return f"{name}-" -def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None) -> tuple[dict[str, str], list[str]]: +def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None, + held: frozenset[str] = frozenset()) -> tuple[dict[str, str], list[str]]: """The registry id each ``@local`` id the plan writes takes, and each problem giving them. What ``imported`` records the folder was imported from takes the id it had there, and an imported task's outputs @@ -167,7 +169,10 @@ def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None) -> t for write in plan.writes: entry = write.source.entry if imported is not None and (source := source_of(imported, plan, write)) is not None: - ids[write.id] = source[0] + if ids.setdefault(write.id, source[0]) != source[0]: + problems.append(f"{plan.path(entry)}: shares its name with what's registered as {ids[write.id]!r}, " + f"so it can't keep the id it was imported as, {source[0]!r}; rename one") + continue imported_tasks.update([entry.id] if write.kind is BundleKind.TASK else []) continue if entry.id in ids or (isinstance(write.source, BundleImage) and write.id != entry.id): @@ -206,9 +211,17 @@ def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None) -> t by_target.setdefault((store, registry_id), []).append(local) if registry_id not in kept and not _REGISTRY_ID.fullmatch(registry_id.replace("__", "_")): problems.append(f"{registry_id!r}: isn't an id a registry takes; rename what it's made from") + copies = {write.id: (write, source_of(imported, plan, write)) for write in plan.writes} if imported else {} for (_, registry_id), locals_ in by_target.items(): - if len(locals_) > 1: + if len(locals_) < 2: + continue + found = [copies.get(local, (None, None)) for local in locals_] + if None in {source for _, source in found} or len({source for _, source in found}) > 1: problems.append(f"{', '.join(sorted(locals_))} would all be registered as {registry_id!r}; rename one") + elif edited := sorted(plan.path(write.source.entry) for write, _ in found if write.id not in held): + problems.append(f"{', '.join(sorted(plan.path(write.source.entry) for write, _ in found))} hold copies of " + f"{registry_id!r} as imported, and {', '.join(edited)} changed it; a registration writes " + "each id once, so undo the change") return ids, problems @@ -216,25 +229,34 @@ def _given(ids: Mapping[str, str], imported_tasks: set[str], plan: Plan, importe """The ``@local`` ids in ``ids`` whose registry ids the rules made, not ``imported``'s record: what isn't an imported write, an imported task's output, or derived from either.""" taken = {write.id for write in plan.writes if source_of(imported, plan, write) is not None} - taken |= {local for local in ids if local.rpartition("/")[0] in imported_tasks} + taken |= {local for local in ids if any(local.startswith(f"{task}/") for task in imported_tasks)} return {local for local in ids if local not in taken and not any(local.startswith(f"{base}__") for base in taken)} -def _sources(plan: Plan, ids: Mapping[str, str], - imported: Imported | None) -> tuple[dict[tuple[str, str], int], set[tuple[str, str]]]: +def _held_as_imported(plan: Plan, imported: Imported | None) -> frozenset[str]: + """The ``@local`` id of each write of ``plan`` that the folder holds as ``imported`` records the import wrote it.""" + if imported is None: + return frozenset() + writes = [write for write in plan.writes if source_of(imported, plan, write) is not None] + root = plan.bundle.bundle.root + now = written_files(root, {where(root, write.source.entry) for write in writes}) + return frozenset(write.id for write in writes if as_imported(imported, plan, write, now)) + + +def _sources(plan: Plan, ids: Mapping[str, str], imported: Imported | None, + held: frozenset[str]) -> tuple[dict[tuple[str, str], int], set[tuple[str, str]]]: """For each write of ``plan``, under its ``@local`` ids, that the folder was imported from, by (store, registry id), - the version it was imported at; and which of them the folder still holds as the import wrote it.""" + the version it was imported at; and which of them the folder still holds as the import wrote it (``held``).""" if imported is None: return {}, set() - now = written_files(plan.bundle.bundle.root) sources, kept = {}, set() for write in plan.writes: if (source := source_of(imported, plan, write)) is None: continue key = (write.kind.store, ids.get(write.id, write.id)) sources[key] = source[1] - if as_imported(imported, plan, write, now): + if write.id in held: kept.add(key) return sources, kept diff --git a/tst/unit/bundle/importer_entities_test.py b/tst/unit/bundle/importer_entities_test.py index e786a80da..4a9fd71e7 100644 --- a/tst/unit/bundle/importer_entities_test.py +++ b/tst/unit/bundle/importer_entities_test.py @@ -25,7 +25,7 @@ from agent_env.bundle.resolve import build_step from agent_env.cli import cli from agent_env.config import get_config -from agent_env.config.runtime import Config +from agent_env.config.runtime import Config, reset_config from agent_env.entity.scopes import FieldScope from agent_env.env.envs.mcp_server import MCPServerEnv from agent_env.env.envs.website import WebsiteEnv @@ -313,6 +313,26 @@ def test_the_store_fingerprint_tells_one_configured_document_store_from_another( assert store_fingerprint() != here and store_fingerprint() == store_fingerprint() +def test_the_store_fingerprint_resolves_env_references_and_ignores_credentials(tmp_path, monkeypatch): + config = tmp_path / "fingerprint.toml" + monkeypatch.setenv("AGENT_ENV_CONFIG", str(config)) + monkeypatch.delenv("AGENT_ENV_DOCUMENT_STORE", raising=False) + + def fingerprint(path, uri): + config.write_text(f'[stores.document]\nimpl = "pkg:Store"\n[stores.document.config]\npath = "{path}"\n' + f'uri = "{uri}"\n') + reset_config() + return store_fingerprint() + + monkeypatch.setenv("DOCUMENT_PATH", "/data/one.db") + one = fingerprint("env:DOCUMENT_PATH", "mongodb://reader:" + "pw-one@db.example.com/agent_env") + monkeypatch.setenv("DOCUMENT_PATH", "/data/two.db") + two = fingerprint("env:DOCUMENT_PATH", "mongodb://reader:" + "pw-one@db.example.com/agent_env") + other_password = fingerprint("env:DOCUMENT_PATH", "mongodb://reader:" + "pw-two@db.example.com/agent_env") + + assert one != two and two == other_password + + def _saved_image(path, url, files): """A one-layer ``docker save`` tarball holding ``files``, stored at ``url``.""" path.write_bytes(saved_image([files], gz=True)) diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index 332f53098..db1e74281 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -9,17 +9,23 @@ from agent_env.a2a_agent import A2AAgent from agent_env.artifact import Artifact, FileArtifact from agent_env.bundle import BundleError +from agent_env.bundle import _import_report as import_report_module from agent_env.bundle import _imported as imported_module from agent_env.bundle import materialize as materialize_module from agent_env.bundle import register as register_module +from agent_env.bundle._imported import Imported, source_of from agent_env.bundle.importer import import_task +from agent_env.bundle.installed import InstalledBundle +from agent_env.bundle.parse import parse_bundle +from agent_env.bundle.plan import plan_bundle from agent_env.bundle.register import REGISTRATIONS, register_bundle -from agent_env.bundle.resolve import build_step +from agent_env.bundle.resolve import build_step, resolve_bundle from agent_env.config import get_config +from agent_env.entity.scopes import FieldScope from agent_env.env.envs.mcp_server import MCPServerEnv from agent_env.store import Filter from agent_env.task import Task -from tst.unit.bundle._support import DIGEST, context_image +from tst.unit.bundle._support import DIGEST, context_image, layout STEPS = [ {"id": "env", "type": "deploy_env", "env_id": "crm-env"}, @@ -157,3 +163,155 @@ def test_an_imported_task_with_an_output_keeps_the_output_s_name(local_stores, t assert Task.get("cli", 2).to_dict()["steps"][1]["cli_artifact_id"] == "crm-cli" assert _written(registered)["cli"] == (2, False) + + +def _pushed(monkeypatch): + monkeypatch.setattr(materialize_module, "push_context_image", + lambda entity_id, context, dockerfile, before=None: (f"registry.example.com/{entity_id}:x", + DIGEST)) + + +@pytest.fixture +def imported_with_user(local_stores, tmp_path, monkeypatch): + """The store's task triage imported with its user-scoped fields: the agent solver written into the folder and named + there, and the secret env var dropped.""" + monkeypatch.setenv("HOME", str(tmp_path)) + MCPServerEnv.put(id="crm-env", docker_image_artifact=context_image(tmp_path, "crm-image"), environment_name="crm") + A2AAgent.put(id="solver", docker_image_artifact=context_image(tmp_path, "solver-image")) + (tmp_path / "crm.json").write_text("{}") + FileArtifact.put("crm-data", description="crm seed", file_path=str(tmp_path / "crm.json")) + Task.put(id="triage", steps=[build_step(dict(step)) for step in STEPS]) + return import_task("triage", tmp_path / "triage", include_scopes={FieldScope.USER}).root + + +def test_a_folder_imported_with_its_user_fields_registers_as_imported(imported_with_user): + step = json.loads((imported_with_user / "tasks/triage.json").read_text())[2] + assert (step["a2a_agent_id"], step.get("a2a_agent_version"), step["env_vars"]) == ("solver", None, {"MODE": "fast"}) + + registered = register_bundle(imported_with_user) + + assert all(reused for _, reused in _written(registered).values()) + + +def test_an_edit_keeps_the_env_vars_the_import_dropped_and_they_are_no_credential_written_out(imported_with_user, + monkeypatch): + monkeypatch.setattr(register_module, "shared_store", lambda store: True) + (imported_with_user / "artifacts/crm-data/crm.json").write_text('{"edited": true}') + + registered = register_bundle(imported_with_user) + + assert _written(registered)["triage"] == (2, False) + assert Task.get("triage", 2).to_dict()["steps"][2]["env_vars"] == {"API_KEY": "abc", "MODE": "fast"} + + +def test_an_imported_skill_takes_back_the_source_the_import_left_out(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + notes = {"name": "notes", "description": "how notes are kept", "object_url": "s3://their-bucket/skills/notes/"} + Task.put(id="skilled", steps=[build_step({"id": "skills", "type": "add_skills", "agent_name": "default-agent", + "skills": [notes]})]) + folder = import_task("skilled", tmp_path / "skilled").root + task_file = folder / "tasks/skilled.json" + task_file.write_text(task_file.read_text().replace("how notes are kept", "how notes are filed")) + + registered = register_bundle(folder) + + assert _written(registered)["skilled"] == (2, False) + skill = Task.get("skilled", 2).to_dict()["steps"][0]["skills"][0] + assert (skill["description"], skill["object_url"]) == ("how notes are filed", notes["object_url"]) + + +def test_envs_sharing_one_stored_image_register_as_imported_and_an_edit_to_one_copy_is_refused(local_stores, tmp_path, + monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + image = context_image(tmp_path, "shared-image") + MCPServerEnv.put(id="crm-env", docker_image_artifact=image, environment_name="crm") + MCPServerEnv.put(id="mail-env", docker_image_artifact=image, environment_name="mail") + Task.put(id="pair", steps=[build_step({"id": "crm", "type": "deploy_env", "env_id": "crm-env"}), + build_step({"id": "mail", "type": "deploy_env", "env_id": "mail-env"})]) + folder = import_task("pair", tmp_path / "pair").root + + kept = register_bundle(folder) + dockerfile = folder / "envs/mail-env/Dockerfile" + dockerfile.write_text(dockerfile.read_text() + "ENV EDITED=1\n") + with pytest.raises(BundleError) as refused: + register_bundle(folder) + + assert all(reused for _, reused in _written(kept).values()) + assert refused.value.problems == ("envs/crm-env, envs/mail-env hold copies of 'shared-image' as imported, and " + "envs/mail-env changed it; a registration writes each id once, so undo the change",) + + +def test_an_image_whose_toml_names_another_dockerfile_is_pushed(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + _pushed(monkeypatch) + files = {"Dockerfile": "FROM python:3.12-slim\nCOPY . /app\n", "app.py": "print(1)\n", + "Dockerfile.slim": "FROM python:3.12-alpine\nCOPY . /app\n"} + MCPServerEnv.put(id="crm-env", docker_image_artifact=context_image(tmp_path, "crm-image", files), + environment_name="crm") + Task.put(id="one", steps=[build_step({"id": "env", "type": "deploy_env", "env_id": "crm-env"})]) + folder = import_task("one", tmp_path / "one").root + toml = folder / "envs/crm-env/env.toml" + assert 'dockerfile = "Dockerfile"' in toml.read_text() + toml.write_text(toml.read_text().replace('dockerfile = "Dockerfile"', 'dockerfile = "Dockerfile.slim"')) + + registered = register_bundle(folder) + + assert _written(registered)["crm-image"] == (2, False) + + +def test_an_imported_task_and_an_env_of_one_name_but_two_ids_keep_their_own(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + MCPServerEnv.put(id="crm", docker_image_artifact=context_image(tmp_path, "crm-image"), environment_name="crm") + Task.put(id="ops/crm", steps=[build_step({"id": "env", "type": "deploy_env", "env_id": "crm"})]) + folder = import_task("ops/crm", tmp_path / "ops").root + + registered = register_bundle(folder) + (folder / "envs/crm-2").rename(folder / "envs/crm") + task_file = folder / "tasks/crm.json" + task_file.write_text(task_file.read_text().replace('"crm-2"', '"crm"')) + with pytest.raises(BundleError) as refused: + register_bundle(folder) + + assert _written(registered) == {"crm-image": (1, True), "crm": (1, True), "ops/crm": (1, True)} + assert refused.value.problems == ("tasks/crm.json: shares its name with what's registered as 'ops-crm', so it " + "can't keep the id it was imported as, 'ops/crm'; rename one",) + + +def test_an_imported_task_s_output_under_a_folder_keeps_its_name(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + MCPServerEnv.put(id="crm-env", docker_image_artifact=context_image(tmp_path, "crm-image"), environment_name="crm") + Task.put(id="cli", steps=[build_step({"id": "env", "type": "deploy_env", "env_id": "crm-env"}), + build_step({"id": "cli", "type": "build_mcp_cli", "env_id": "crm-env", + "command_name": "crm", "cli_artifact_id": "tools/crm-cli"})]) + folder = import_task("cli", tmp_path / "cli").root + (folder / "tasks/cli.json").write_text((folder / "tasks/cli.json").read_text().replace('"crm"', '"crm2"')) + + registered = register_bundle(folder) + + assert Task.get("cli", 2).to_dict()["steps"][1]["cli_artifact_id"] == "tools/crm-cli" + assert _written(registered)["cli"] == (2, False) + + +def test_registering_reads_only_the_files_of_what_the_import_wrote(imported, monkeypatch): + read = [] + hash_file = import_report_module.file_sha256 + monkeypatch.setattr(import_report_module, "file_sha256", lambda path: read.append(path) or hash_file(path)) + layout(imported, {".venv/lib/big.py": "x = 1\n", "notes/todo.md": "later\n"}) + + register_bundle(imported) + + assert read and not [path for path in read if ".venv" in path.parts or "notes" in path.parts] + + +def test_an_installed_bundle_s_agent_takes_no_source_from_the_folder_s_record(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + kit_root = layout(tmp_path / "site" / "kit", {"agents/solver/Dockerfile": "FROM scratch\n"}) + kit = InstalledBundle("kit", "demo-dist", "1.0", "demo", kit_root, None, None) + folder = layout(tmp_path / "triage", {"tasks/t.json": json.dumps([ + {"id": "agent", "type": "deploy_agent", "a2a_agent_id": "@local/demo-dist/kit/solver"}])}) + plan = plan_bundle(resolve_bundle(parse_bundle(folder)), installed=lambda: (kit,)) + record = Imported({"agents/solver": ("solver", 3)}, {("agents/solver", "image"): ("solver-image", 3)}, {}, {}) + + installed = [write for write in plan.writes if write.source.entry in plan.origins] + + assert installed and [source_of(record, plan, write) for write in installed] == [None] * len(installed) From 8e8eba1af4785671dd155aec6077293bc3113478 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 16:52:01 -0700 Subject: [PATCH 12/22] refactor(bundle): let the ledger say how a registration writes, and read each pinned version one way - materialize takes the ledger instead of a Registering wrapper: Ledger.pins_references now also decides whether tasks and evals are written pinned, and Ledger.pushes_images whether built images are pushed. - Tasks and evals pin through one helper, reading the versions straight from what was written rather than a copy kept in step by hand. An eval ref whose store version the plan didn't read now floats instead of raising, as a task's does. - The kind-to-store table is gone: an EntityKind's value is its store's key. - The registry-id check no longer rewrites "__", which the id pattern already allows. - translated() builds its fields in _translations, beside _KEPT, the fields that hold no @local id; a test fails when a Plan field is in neither. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/ledger.py | 3 ++ src/agent_env/bundle/materialize.py | 68 ++++++++++++----------------- src/agent_env/bundle/register.py | 41 ++++++++++------- tst/unit/bundle/register_test.py | 14 +++++- 4 files changed, 69 insertions(+), 57 deletions(-) diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index d97bcc852..573b86d8a 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -85,6 +85,9 @@ class Ledger: # Whether each write lands on exactly the version its check found next, so one that loses a race to another # writer raises ConcurrentModificationError rather than add a version after the other's. strict = False + # Whether each image the bundle builds is built here and pushed to the configured image store, for whatever reads the + # store, rather than written as its build context for each deploy to build. + pushes_images = False def __init__(self, store: DocumentStore, plan: Plan): self._store = store diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 7f42c5049..1382a4b7b 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -17,7 +17,7 @@ from __future__ import annotations import copy -from collections.abc import Callable, Iterator +from collections.abc import Callable, Iterator, Mapping from contextlib import contextmanager, nullcontext from dataclasses import dataclass from typing import Any @@ -42,14 +42,7 @@ from .ledger import Ledger, locked_ids, materializing from .parse import BundleError, BundleKind from .plan import Plan, Write, artifact_writer, env_writer, unpinned_store_refs -from .resolve import BuiltImage, BundleImage, ReferencedImage, Reference, build_step - -@dataclass(frozen=True) -class Registering: - """How a registration writes a plan: through ``ledger``, which keeps its rows in the target store, each task and - eval naming the version of each entity it references, and each image the bundle builds built here and pushed.""" - - ledger: Ledger +from .resolve import BuiltImage, BundleImage, ReferencedImage, build_step @dataclass(frozen=True) @@ -87,7 +80,7 @@ def materialize( on_build: Callable[[Write], None] | None = None, on_write: Callable[[Materialized], None] | None = None, build_here: frozenset[str] = frozenset(), - registering: Registering | None = None, + ledger: Ledger | None = None, ) -> Materialization: """Write ``plan``'s entities, tasks and evals. The first write that fails stops it, and every earlier one stays: they're in the ledger, so the next run reuses them. ``on_wait`` is called when another run @@ -101,12 +94,14 @@ def materialize( Each write gets the version it would reuse or the store's next, which another run can take first. A step reading what the run would write first isn't preflighted, since the store doesn't hold it yet. - ``registering`` writes as a registration does (``Registering``).""" + ``ledger`` keeps the rows, this run's in the ``@local`` namespace's store unless given; it also says whether each + task and eval names the versions it reads (``Ledger.pins_references``) and whether each image the bundle builds is + built here and pushed (``Ledger.pushes_images``), as a registration's does.""" _refuse_unwritable(plan) if not namespace_routing_enabled(): raise RuntimeError("materializing a bundle needs namespace routing, which the agent-env CLI turns on; " "call it inside agent_env.store.routing.namespace_routing()") - ledger = registering.ledger if registering is not None else Ledger.for_plan(plan) + ledger = ledger if ledger is not None else Ledger.for_plan(plan) entities = [write for write in plan.writes if write.kind not in (BundleKind.TASK, BundleKind.EVAL)] tasks = [write for write in plan.writes if write.kind is BundleKind.TASK] evals = [write for write in plan.writes if write.kind is BundleKind.EVAL] @@ -130,18 +125,18 @@ def through_ledger(write: Write, write_fn: Callable[[], int]) -> None: with nullcontext() if dry_run else materializing(locked_ids(plan), on_wait): for write in entities: here = write.id in build_here and not dry_run - if registering is not None and isinstance(write.source, BuiltImage) and write.id not in plan.published: + if ledger.pushes_images and isinstance(write.source, BuiltImage) and write.id not in plan.published: through_ledger(write, lambda: _write_pushed_image(plan, write, on_build)) else: through_ledger(write, lambda: _write_entity(plan, write, here, on_build)) if here and done[_key(write)].reused: _build_reused(plan, write, done[_key(write)].version, on_build) unwritten = {_key(item.write) for item in done.values() if not item.reused} if dry_run else set() - versions = {key: item.version for key, item in done.items()} if registering is not None else None + written = done if ledger.pins_references else None built, problems, unchecked = {}, [], [] for write in tasks: with _noted(plan, write, "preflighting"): - built[write.id] = _build(write, plan, versions) + built[write.id] = _build(write, plan, written) found, skipped = _preflight(plan, write, built[write.id], unwritten) problems.extend(f"{_path(plan, write)}: {problem}" for problem in found) unchecked.extend((write, step) for step in skipped) @@ -150,9 +145,7 @@ def through_ledger(write: Write, write_fn: Callable[[], int]) -> None: for write in tasks: through_ledger(write, lambda: Task.put(id=write.id, steps=built[write.id].steps).version) for write in evals: - if versions is not None: - versions.update({key: item.version for key, item in done.items()}) - through_ledger(write, lambda: _write_eval(plan, write, versions)) + through_ledger(write, lambda: _write_eval(plan, write, written)) return Materialization(plan, tuple(done[_key(write)] for write in plan.writes), tuple(unchecked)) @@ -310,26 +303,20 @@ def _pinned(plan: Plan, write: Write, refs: tuple[EntityRef, ...]) -> Any: return config -def _write_eval(plan: Plan, write: Write, versions: dict[tuple[str, str], int] | None = None) -> int: +def _write_eval(plan: Plan, write: Write, written: Mapping[tuple[str, str], Materialized] | None = None) -> int: """An unpinned task ref, the bundle's own or a store's, is written without a version, so the eval runs its latest; - given the ``versions`` written, as a registration is, each names the version it reads.""" - tasks = [EvalTask(ref.id, ref.version if versions is None else _version(plan, ref, versions)) + given what was ``written``, as a registration is, each names the version it reads.""" + tasks = [EvalTask(ref.id, ref.version if written is None or ref.version is not None + else _read_version(plan, written, ref.kind, ref.id, own=ref.local is not None)) for ref in write.source.references] return Eval.put(id=write.id, tasks=tasks).version -def _version(plan: Plan, ref: Reference, versions: dict[tuple[str, str], int]) -> int: - """The version ``ref`` reads: its own, the one written for the bundle's entity it names, or the store's latest the - plan read.""" - if ref.version is not None: - return ref.version - if ref.local is not None: - return versions[_KINDS[ref.kind], ref.id] - return plan.store_latest[ref.kind, ref.id] - - -# The store each kind a reference names is written to, by the key ``done`` holds it under. -_KINDS = {EntityKind.ENV: "env", EntityKind.AGENT: "agent", EntityKind.ARTIFACT: "artifact", EntityKind.TASK: "task"} +def _read_version(plan: Plan, written: Mapping[tuple[str, str], Materialized], kind: EntityKind, id: str, *, + own: bool) -> int | None: + """The version a reference naming none reads of ``kind`` ``id``: the one ``written`` for the bundle's ``own`` + entity, else the store's latest the plan read, or None when the plan read none.""" + return written[kind.value, id].version if own else plan.store_latest.get((kind, id)) # The writers this release has, by kind; any other kind is refused before anything is written. Tasks are @@ -370,17 +357,16 @@ def _noted(plan: Plan, write: Write, doing: str) -> Iterator[None]: raise -def _build(write: Write, plan: Plan, versions: dict[tuple[str, str], int] | None) -> Task: - """The task ``write`` puts: as resolved, or given the ``versions`` written, with each reference that takes a version +def _build(write: Write, plan: Plan, written: Mapping[tuple[str, str], Materialized] | None) -> Task: + """The task ``write`` puts: as resolved, or given what was ``written``, with each reference that takes a version naming the one it reads.""" - steps = write.source.config if versions is None else pinned_steps(plan, write, versions) + steps = write.source.config if written is None else pinned_steps(plan, write, written) return Task(id=write.id, version=None, steps=[build_step(step) for step in steps]) -def pinned_steps(plan: Plan, write: Write, versions: dict[tuple[str, str], int]) -> list[dict]: +def pinned_steps(plan: Plan, write: Write, written: Mapping[tuple[str, str], Materialized]) -> list[dict]: """A copy of the task ``write``'s steps with each reference it reads that names no version, where its field takes - one, naming the version it reads: the one written for the bundle's own entity, or the store's latest the plan - read.""" + one, naming the version it reads (``_read_version``).""" steps = copy.deepcopy(write.source.config) own = {(ref.kind, ref.id) for ref in write.source.references if ref.local is not None} registry = get_task_step_registry() @@ -388,8 +374,8 @@ def pinned_steps(plan: Plan, write: Write, versions: dict[tuple[str, str], int]) for site in ref_sites(registry[step["type"]].entity_refs or (), step): if site.ref.role is not RefRole.INPUT or site.version_key is None or site.version is not None: continue - key = (site.ref.kind, site.value) - version = versions[_KINDS[site.ref.kind], site.value] if key in own else plan.store_latest.get(key) + own_entity = (site.ref.kind, site.value) in own + version = _read_version(plan, written, site.ref.kind, site.value, own=own_entity) if version is not None: site.rewrite(site.value, version) return steps diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 959671d1b..06a3455ff 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -42,7 +42,7 @@ from ._secrets import credentials_in from .installed import installed_bundles from .ledger import Check, Ledger -from .materialize import Materialization, Materialized, Registering, materialize +from .materialize import Materialization, Materialized, materialize from .parse import Bundle, BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle, plan_entities from .run import _label @@ -103,7 +103,7 @@ def register_bundle( raise BundleError(problems) say = on_progress or (lambda line: None) materialization = materialize( - registered, dry_run=dry_run, registering=Registering(ledger), + registered, dry_run=dry_run, ledger=ledger, on_wait=lambda: say("waiting for another agent-env run to finish writing this bundle's ids"), on_build=lambda write: say(f" {_label(registered, write)}: building with docker and pushing"), on_write=lambda done: say(f" {registered_line(registered, done)}")) @@ -188,7 +188,7 @@ def registry_ids(plan: Plan, prefix: str) -> tuple[dict[str, str], list[str]]: for local, registry_id in ids.items(): for store in stores.get(local, ()): by_target.setdefault((store, registry_id), []).append(local) - if not _REGISTRY_ID.fullmatch(registry_id.replace("__", "_")): + if not _REGISTRY_ID.fullmatch(registry_id): problems.append(f"{registry_id!r}: isn't an id a registry takes; rename what it's made from") for (_, registry_id), locals_ in by_target.items(): if len(locals_) > 1: @@ -214,8 +214,18 @@ def _slug(name: str) -> str: return _NOT_IN_ID.sub("-", ascii_name.replace("__", "-")).strip("-._") +# The fields of a Plan that hold no ``@local`` id, so a translated plan keeps them: the store's latest versions, by store +# id, and the agent ``--agent`` names, which a registration plans without. +_KEPT = frozenset({"store_latest", "agent"}) + + def translated(plan: Plan, ids: Mapping[str, str]) -> Plan: """``plan`` writing each entity under its registry id, and naming it so wherever the plan's documents do.""" + return replace(plan, **_translations(plan, ids)) + + +def _translations(plan: Plan, ids: Mapping[str, str]) -> dict[str, Any]: + """Each field of ``plan`` but those it keeps (``_KEPT``), with each ``@local`` id in it its registry id.""" def entity(entry: BundleEntry) -> BundleEntry: if entry not in entries: @@ -250,16 +260,16 @@ def source_of(source: ResolvedEntry | BundleImage) -> ResolvedEntry | BundleImag resolved_bundle = ResolvedBundle(bundle, tuple(sources.get(entity(entry.entry)) or resolved(entry) for entry in plan.bundle.entries), tuple(image(source) for source in plan.bundle.images)) - return replace( - plan, bundle=resolved_bundle, writes=writes, - tasks=tuple(sources[entity(task.entry)] for task in plan.tasks), - evals=tuple(sources[entity(item.entry)] for item in plan.evals), - store_refs=tuple(reference(ref) for ref in plan.store_refs), - origins={entity(entry): origin for entry, origin in plan.origins.items()}, - published={ids.get(id, id): publication for id, publication in plan.published.items()}, - aliases={alias: _aliased(use, ids) for alias, use in plan.aliases.items()}, - env_aliases={alias: _aliased(use, ids) for alias, use in plan.env_aliases.items()}, - ) + return { + "bundle": resolved_bundle, "writes": writes, + "tasks": tuple(sources[entity(task.entry)] for task in plan.tasks), + "evals": tuple(sources[entity(item.entry)] for item in plan.evals), + "store_refs": tuple(reference(ref) for ref in plan.store_refs), + "origins": {entity(entry): origin for entry, origin in plan.origins.items()}, + "published": {ids.get(id, id): publication for id, publication in plan.published.items()}, + "aliases": {alias: _aliased(use, ids) for alias, use in plan.aliases.items()}, + "env_aliases": {alias: _aliased(use, ids) for alias, use in plan.env_aliases.items()}, + } def _aliased(use: AliasUse, ids: Mapping[str, str]) -> AliasUse: @@ -279,12 +289,13 @@ def _renamed(value: Any, ids: Mapping[str, str]) -> Any: class RegistrationLedger(Ledger): """A registration's rows, in the target store: each names the bundle that wrote it, by its identity, and the commit - and state of the folder it was written from. A task or eval is hashed with the versions it names, and each write - lands on exactly the version its check found next.""" + and state of the folder it was written from. A task or eval is hashed with the versions it names, each write lands + on exactly the version its check found next, and each image the bundle builds is built here and pushed.""" collection = REGISTRATIONS pins_references = True strict = True + pushes_images = True def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None, adopt: frozenset[str] = frozenset()): diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 305cb730a..136647b27 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -4,6 +4,7 @@ import json import subprocess +from dataclasses import fields from pathlib import Path import pytest @@ -23,7 +24,9 @@ from agent_env.entity.refs import EntityRef, RefRole from agent_env.env.env import Env from agent_env.eval import Eval -from agent_env.bundle.parse import BundleKind +from agent_env.bundle.parse import BundleKind, parse_bundle +from agent_env.bundle.plan import Plan, plan_bundle +from agent_env.bundle.resolve import resolve_bundle from agent_env.store import ConcurrentModificationError, Filter from agent_env.store.base import NotFoundError from agent_env.store.document_store import expecting_version @@ -211,6 +214,15 @@ def test_entries_whose_names_make_one_id_register_in_their_own_stores(home): ("artifact", "triage-echo"), ("task", "triage-echo")} +def test_a_translated_plan_renames_every_field_of_a_plan_but_those_that_hold_no_local_id(home): + plan = plan_bundle(resolve_bundle(parse_bundle(layout(home / "triage", TRIAGE))), tasks=["t"]) + + translations = register_module._translations(plan, {}) + + assert translations.keys() | register_module._KEPT == {field.name for field in fields(Plan)} + assert not translations.keys() & register_module._KEPT + + def test_a_bad_prefix_is_refused(home): with pytest.raises(BundleError) as refused: register_bundle(layout(home / "triage", TRIAGE), prefix="Team/") From b70a3bee755a1a76d7966991e913803c605097eb Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 16:58:03 -0700 Subject: [PATCH 13/22] fix(bundle): register a pulled image as imported, and keep a skill source the folder gives - The import records each image it names by registry ref with its stored id and version, so registering an untouched folder reuses it rather than writing it, its env and its task anew. - A list item's left-out fields come back together, and only when the item sets none of them, so a new object_url or a switch to body or skill_artifact_id is never undone by the stored s3_url. Matching by name no longer applies when the item sets a field its stored item doesn't. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_import_report.py | 4 ++ src/agent_env/bundle/_imported.py | 47 ++++++++++++++---------- src/agent_env/bundle/importer.py | 5 ++- tst/unit/bundle/register_imports_test.py | 36 ++++++++++++++++++ 4 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index a096b7f82..6dbf1e552 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -101,6 +101,8 @@ class Pulled: folder: str key: str image: str + artifact: str # the stored image's id and version + version: int why: str | None = None # why it's pulled though it has a build context: the context can't be written @@ -181,6 +183,8 @@ def provenance(source: dict, imported: BundleImport, root: Path) -> str: "images": [{"folder": image.folder, "key": image.key, "id": image.artifact, "version": image.version, "dockerfile": image.pruned.dockerfile, "dockerfile_found": image.found} for image in imported.images], + "pulled": [{"folder": pull.folder, "key": pull.key, "id": pull.artifact, "version": pull.version} + for pull in imported.pulled], "pinned": [{"where": pin.where, "kind": pin.kind.value, "id": pin.id, "version": pin.version, "why": pin.why} for pin in imported.pinned], "files": written_files(root), diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index bd1d4a969..a20c25c64 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -29,7 +29,8 @@ @dataclass(frozen=True) class Imported: """What an import recorded: each entity folder, task file and eval file's store id and version, each image's by its - folder and toml key, with the Dockerfile it's built from, and each file as it wrote it.""" + folder and toml key, built or pulled, with the Dockerfile a built one is built from, and each file as it wrote + it.""" entities: Mapping[str, tuple[str, int]] images: Mapping[tuple[str, str], tuple[str, int]] @@ -55,7 +56,8 @@ def read_imported(root: Path) -> tuple[Imported | None, str | None]: entities |= {item["file"]: (item["id"], item["version"]) for item in record.get("tasks", [])} if (stored_eval := record.get("eval")) is not None: entities[stored_eval["file"]] = (stored_eval["id"], stored_eval["version"]) - images = {(item["folder"], item["key"]): (item["id"], item["version"]) for item in record.get("images", [])} + images = {(item["folder"], item["key"]): (item["id"], item["version"]) + for item in [*record.get("images", []), *record.get("pulled", [])]} dockerfiles = {(item["folder"], item["key"]): item.get("dockerfile") for item in record.get("images", [])} return Imported(entities, images, record.get("files", {}), dockerfiles), None @@ -139,16 +141,20 @@ def _given_back(step: dict, origin: dict, cls: type) -> list[str]: """Give ``step`` back what the import left out of ``origin``, in place; the path of each field it took.""" user = [path for path, scope in cls.field_scopes.items() if scope is FieldScope.USER] repinned = _repinned(step, origin, cls) - given = [] + given, lists = [], {} for path in user: if path in repinned: continue if "[]" in path: - given += _given_back_items(step, origin, path, user) + at, _, field = path.partition("[].") + if "[]" not in field: + lists.setdefault(at, []).append(field) continue for _, holder, key in field_sites(origin, path): if holder[key] not in ("", [], {}) and _graft(step, path.split("."), holder[key]): given.append(path) + for at, fields in lists.items(): + given += _given_back_items(step, origin, at, fields) scrubbed = copy.deepcopy(origin) scrub(scrubbed) return given + _unscrubbed(step, origin, scrubbed) @@ -163,33 +169,36 @@ def _repinned(step: dict, origin: dict, cls: type) -> set[str]: if site.version_key is not None and held.get(site.path, site.value) != site.value} -def _given_back_items(step: dict, origin: dict, path: str, user: list[str]) -> list[str]: - """Give each item of the list ``path`` names, ``[].``, its field from the stored item it was written - from: the one it equals but for the list's user-scoped fields, else the one alone in sharing its name.""" - at, _, field = path.partition("[].") - if "[]" in field: - return [] +def _given_back_items(step: dict, origin: dict, at: str, fields: list[str]) -> list[str]: + """Give each item of the list ``at`` the ``fields`` the import left out of it, from the stored item it was written + from (``_source_item``): all of them, or none when the item sets one, since together they can be one choice, such as + a source under either of its names.""" items, stored = _at(step, at), _at(origin, at) if not isinstance(items, list) or not isinstance(stored, list): return [] - left_out = {other.partition("[].")[2].split(".")[0] for other in user if other.startswith(f"{at}[].")} + left_out = {field.split(".")[0] for field in fields} given = [] for index, item in enumerate(items): + if not isinstance(item, dict) or any(_at(item, field) is not None for field in fields): + continue if (source := _source_item(item, stored, left_out)) is None: continue - for _, holder, key in field_sites(source, field): - if holder[key] not in ("", [], {}) and _graft(item, field.split("."), holder[key]): - given.append(f"{at}[{index}].{field}") + for field in fields: + for _, holder, key in field_sites(source, field): + if holder[key] not in ("", [], {}) and _graft(item, field.split("."), holder[key]): + given.append(f"{at}[{index}].{field}") return given -def _source_item(item: object, stored: list, left_out: set[str]) -> dict | None: - if not isinstance(item, dict): - return None +def _source_item(item: dict, stored: list, left_out: set[str]) -> dict | None: + """The stored item ``item`` was written from: the one it equals but for the ``left_out`` fields, else the one alone + in sharing its name, when ``item`` sets nothing that one doesn't.""" candidates = [one for one in stored if isinstance(one, dict)] - same = [one for one in candidates if _bare(one, left_out) == _bare(item, left_out)] + bare = _bare(item, left_out) + same = [one for one in candidates if _bare(one, left_out) == bare] if not same and item.get("name") is not None: - same = [one for one in candidates if one.get("name") == item["name"]] + same = [one for one in candidates + if one.get("name") == item["name"] and bare.keys() <= _bare(one, left_out).keys()] return same[0] if len(same) == 1 else None diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index a46c4f572..30180a89c 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -266,7 +266,7 @@ def image(self, out: _EntityOut, artifact: DockerImageArtifact, key: str) -> str out.images.append(job) return job.toml if names_registry(artifact.image_name): - out.pulled.append(Pulled(out.folder, key, artifact.image_name)) + out.pulled.append(Pulled(out.folder, key, artifact.image_name, artifact.id, artifact.version)) return {"ref": artifact.image_name} why = (f"it's stored only as a docker save tarball, under {artifact.image_name}, which names no registry" if artifact.tar_gz_object_url else "it records neither a build context nor a registry to pull it from") @@ -346,7 +346,8 @@ def _image_left_in_store(self, job: ImageJob) -> None: if names_registry(job.artifact.image_name): job.toml.clear() job.toml["ref"] = job.artifact.image_name - self.pulled.append(Pulled(job.folder, job.key, job.artifact.image_name, job.problem)) + self.pulled.append(Pulled(job.folder, job.key, job.artifact.image_name, job.artifact.id, + job.artifact.version, job.problem)) return key = (EntityKind.ARTIFACT, job.artifact.id, job.artifact.version) try: diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index db1e74281..ccfc721c5 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -8,6 +8,8 @@ from agent_env.a2a_agent import A2AAgent from agent_env.artifact import Artifact, FileArtifact +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.artifact.store import get_artifact_store from agent_env.bundle import BundleError from agent_env.bundle import _import_report as import_report_module from agent_env.bundle import _imported as imported_module @@ -315,3 +317,37 @@ def test_an_installed_bundle_s_agent_takes_no_source_from_the_folder_s_record(lo installed = [write for write in plan.writes if write.source.entry in plan.origins] assert installed and [source_of(record, plan, write) for write in installed] == [None] * len(installed) + + +def test_an_env_whose_image_is_pulled_from_its_registry_registers_as_imported(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + image = get_artifact_store().put_document(DockerImageArtifact( + id="crm-image", description="crm", image_name="registry.example.com/crm:v1", + tar_gz_s3_url="file:///nowhere/crm-image.tar.gz")) + MCPServerEnv.put(id="crm-env", docker_image_artifact=image, environment_name="crm") + Task.put(id="pulled", steps=[build_step({"id": "env", "type": "deploy_env", "env_id": "crm-env"})]) + folder = import_task("pulled", tmp_path / "pulled").root + + registered = register_bundle(folder) + + assert _written(registered) == {"crm-image": (1, True), "crm-env": (1, True), "pulled": (1, True)} + + +@pytest.mark.parametrize("source", [{"object_url": "s3://their-bucket/skills/notes-v2/"}, + {"body": "Keep notes in notes.md."}]) +def test_a_skill_the_folder_gives_a_source_of_its_own_keeps_it(local_stores, tmp_path, monkeypatch, source): + monkeypatch.setenv("HOME", str(tmp_path)) + notes = {"name": "notes", "description": "how notes are kept", "object_url": "s3://their-bucket/skills/notes/"} + Task.put(id="skilled", steps=[build_step({"id": "skills", "type": "add_skills", "agent_name": "default-agent", + "skills": [notes]})]) + folder = import_task("skilled", tmp_path / "skilled").root + task_file = folder / "tasks/skilled.json" + steps = json.loads(task_file.read_text()) + steps[0]["skills"][0].update(source) + task_file.write_text(json.dumps(steps)) + + register_bundle(folder) + + skill = Task.get("skilled", 2).to_dict()["steps"][0]["skills"][0] + assert {key: skill.get(key) for key in ("object_url", "s3_url", "body")} == { + "object_url": source.get("object_url"), "s3_url": source.get("object_url"), "body": source.get("body")} From 8df98e605e8ad070ef455651364891fa385a70d9 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 22:31:18 -0700 Subject: [PATCH 14/22] test(bundle): register the echo-mcp example against a real registry, and through one that's down Builds and pushes the env's image to the local registry, runs the registered task by its id to a passing score, and checks registering again writes nothing. A registry that can't be reached fails the registration before the image is built or named, and a rerun once it's back finishes. Co-Authored-By: Claude Opus 5.5 --- tst/integration/cli/bundle_register_test.py | 119 ++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tst/integration/cli/bundle_register_test.py diff --git a/tst/integration/cli/bundle_register_test.py b/tst/integration/cli/bundle_register_test.py new file mode 100644 index 000000000..e0239a9ac --- /dev/null +++ b/tst/integration/cli/bundle_register_test.py @@ -0,0 +1,119 @@ +"""``agent-env bundle register`` on the echo-mcp example agent-env ships, into the local stores and this machine's local +registry: its env's image is built with Docker and pushed, the registered task runs by its id and passes, and +registering again writes nothing. A registry it can't reach fails the registration before it names the image anywhere, +and once the registry is back a rerun finishes. Needs a Docker daemon and the local registry.""" + +import json +import logging +import shutil +import uuid +from pathlib import Path + +import pytest +from click.testing import CliRunner + +import agent_env +from agent_env.artifact import DockerImageArtifact +from agent_env.artifact.store import reset_artifact_store +from agent_env.cli import cli +from agent_env.config import configure, reset_config +from agent_env.env.bootstrap import GATEWAY, SERVICE_DB, ensure_default_envs +from agent_env.env.env import Env +from agent_env.store.base import NotFoundError +from agent_env.task import Task + +pytestmark = [pytest.mark.integration, pytest.mark.int_test_slow] + +ECHO = Path(agent_env.__file__).parent / "examples" / "echo-mcp" +IMAGE = "envs/echo (Dockerfile image)" +# A registry nothing serves: the plain OCI store, since the local registry's would start one on its port. +UNREACHABLE = """\ +[stores.image] +impl = "agent_env.store.image_store:OciRegistryImageStore" +[stores.image.config] +registry_host = "127.0.0.1:9" +""" + + +@pytest.fixture +def bundle(monkeypatch, tmp_path): + """A copy of the echo-mcp example, with the local stores under this test's folder, and a prefix of its own, so + the registry, which outlives the test, holds nothing under its ids. Whatever ran, no sandbox work folder may be + left behind.""" + sandboxes = tmp_path / "sandboxes" + # HOME stays: docker's credential helper (the macOS keychain, for one) can hang a build's base-image lookup + # when HOME moves. + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "state")) + monkeypatch.setenv("AGENT_ENV_DOCUMENT_STORE", "local") + monkeypatch.setenv("AGENT_ENV_OBJECT_STORE", "local") + monkeypatch.setenv("AGENT_ENV_LOCAL_SANDBOX_DIR", str(sandboxes)) + configure() + reset_artifact_store() + logging.disable(logging.CRITICAL) # pytest's live logging would take CliRunner's stdout + root = tmp_path / "echo" + shutil.copytree(ECHO, root) + try: + yield root, f"it-{uuid.uuid4().hex[:8]}-" + assert not sandboxes.exists() or not any(sandboxes.iterdir()), "a run left a sandbox work folder" + finally: + logging.disable(logging.NOTSET) + reset_artifact_store() + reset_config() + + +def _register(root, prefix): + return CliRunner().invoke(cli, ["bundle", "register", str(root), "--prefix", prefix]) + + +def test_a_registered_bundle_s_image_is_pushed_its_task_runs_by_id_and_registering_again_writes_nothing(bundle, + tmp_path): + root, prefix = bundle + + registered = _register(root, prefix) + + assert registered.exit_code == 0, registered.output + assert registered.output.count("building with docker and pushing") == 1, registered.output + assert f"{IMAGE}: building with docker and pushing" in registered.output, registered.output + image = DockerImageArtifact.get(f"{prefix}echo__env_image") + assert image.image_digest.startswith("sha256:") and image.build_context_object_url, image + assert Env.get(f"{prefix}echo").docker_image_artifact.id == image.id + + # The gateway and service-db envs a stored task's deploy needs, which `agent-env up` sets up in a store this new. + ensure_default_envs((SERVICE_DB, GATEWAY)) + ran = CliRunner().invoke(cli, ["task", "run", "--id", f"{prefix}smoke", "--output-dir", str(tmp_path / "out")]) + again = _register(root, prefix) + + assert ran.exit_code == 0, ran.output + [context] = (tmp_path / "out").iterdir() + assert json.loads(context.read_text())["metadata"]["verifications"]["echoes"]["score"] == 1.0, ran.output + assert again.exit_code == 0, again.output + assert "Nothing to write: every version is registered already." in again.output, again.output + assert "building with docker" not in again.output, again.output + + +def test_a_registry_it_cant_reach_fails_it_before_the_image_is_named_and_a_rerun_once_it_is_back_finishes( + bundle, tmp_path, monkeypatch): + root, prefix = bundle + config = tmp_path / "unreachable.toml" + config.write_text(UNREACHABLE) + monkeypatch.delenv("AGENT_ENV_IMAGE_STORE", raising=False) + with monkeypatch.context() as unreachable: + unreachable.setenv("AGENT_ENV_CONFIG", str(config)) + reset_config() + + failed = _register(root, prefix) + + assert failed.exit_code != 0, failed.output + assert "couldn't read its manifest from 127.0.0.1:9" in failed.output, failed.output + assert "building with docker" not in failed.output, failed.output + for read in (lambda: Env.get(f"{prefix}echo"), lambda: Task.get(f"{prefix}smoke")): + with pytest.raises(NotFoundError): + read() + reset_config() + + finished = _register(root, prefix) + + assert finished.exit_code == 0, finished.output + assert f"{IMAGE}: building with docker and pushing" in finished.output, finished.output + assert Task.get(f"{prefix}smoke").version == 1 From 5bc8d33115a39c76bb9ffc0f4daab9e837a21e13 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 23:17:21 -0700 Subject: [PATCH 15/22] fix(bundle): translate main's alias_uses, and keep the register integration tests on the local registry main folded Plan's aliases and env_aliases into alias_uses, by kind, so a registered plan renames each of them there. The integration tests now use the local registry whatever AGENT_ENV_CONFIG names, so they never push to a hosted one. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/register.py | 4 ++-- tst/integration/cli/bundle_register_test.py | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 06a3455ff..2cdeb4672 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -267,8 +267,8 @@ def source_of(source: ResolvedEntry | BundleImage) -> ResolvedEntry | BundleImag "store_refs": tuple(reference(ref) for ref in plan.store_refs), "origins": {entity(entry): origin for entry, origin in plan.origins.items()}, "published": {ids.get(id, id): publication for id, publication in plan.published.items()}, - "aliases": {alias: _aliased(use, ids) for alias, use in plan.aliases.items()}, - "env_aliases": {alias: _aliased(use, ids) for alias, use in plan.env_aliases.items()}, + "alias_uses": {kind: {alias: _aliased(use, ids) for alias, use in uses.items()} + for kind, uses in plan.alias_uses.items()}, } diff --git a/tst/integration/cli/bundle_register_test.py b/tst/integration/cli/bundle_register_test.py index e0239a9ac..2e1ddb9c4 100644 --- a/tst/integration/cli/bundle_register_test.py +++ b/tst/integration/cli/bundle_register_test.py @@ -37,9 +37,9 @@ @pytest.fixture def bundle(monkeypatch, tmp_path): - """A copy of the echo-mcp example, with the local stores under this test's folder, and a prefix of its own, so - the registry, which outlives the test, holds nothing under its ids. Whatever ran, no sandbox work folder may be - left behind.""" + """A copy of the echo-mcp example, with the local stores under this test's folder and the local registry, whatever + the config names, and a prefix of its own, so the registry, which outlives the test, holds nothing under its ids. + Whatever ran, no sandbox work folder may be left behind.""" sandboxes = tmp_path / "sandboxes" # HOME stays: docker's credential helper (the macOS keychain, for one) can hang a build's base-image lookup # when HOME moves. @@ -47,6 +47,7 @@ def bundle(monkeypatch, tmp_path): monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "state")) monkeypatch.setenv("AGENT_ENV_DOCUMENT_STORE", "local") monkeypatch.setenv("AGENT_ENV_OBJECT_STORE", "local") + monkeypatch.setenv("AGENT_ENV_IMAGE_STORE", "local") monkeypatch.setenv("AGENT_ENV_LOCAL_SANDBOX_DIR", str(sandboxes)) configure() reset_artifact_store() @@ -97,9 +98,9 @@ def test_a_registry_it_cant_reach_fails_it_before_the_image_is_named_and_a_rerun root, prefix = bundle config = tmp_path / "unreachable.toml" config.write_text(UNREACHABLE) - monkeypatch.delenv("AGENT_ENV_IMAGE_STORE", raising=False) with monkeypatch.context() as unreachable: unreachable.setenv("AGENT_ENV_CONFIG", str(config)) + unreachable.delenv("AGENT_ENV_IMAGE_STORE") reset_config() failed = _register(root, prefix) From c8a21da2deacbe1606242404d2aedc93f5b2a381 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 23:23:12 -0700 Subject: [PATCH 16/22] fix(bundle): read an imported folder as its writer does, and record an unchanged imported write once - Registration compares each imported write with the files its writer reads: links inside the bundle are followed, so a linked folder in an image's context is an edit to it, and what the OS leaves behind, such as .DS_Store, isn't. - An unchanged imported write is adopted only when the ledger has no row of this bundle for it at that version and digest, so registering again adds no rows. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_import_report.py | 16 ++++++++------ src/agent_env/bundle/register.py | 27 +++++++++++++++++++----- tst/unit/bundle/register_imports_test.py | 17 +++++++++++++++ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index 6dbf1e552..e611d9544 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -7,7 +7,6 @@ import json import posixpath import unicodedata -from collections.abc import Iterable from dataclasses import dataclass from datetime import datetime, timezone from importlib.metadata import version as package_version @@ -201,18 +200,21 @@ def store_fingerprint() -> str: return "sha256:" + hashlib.sha256(text.encode()).hexdigest() -def written_files(root: Path, within: Iterable[str] | None = None) -> dict[str, str]: - """Each file under ``root``, or under its paths ``within``, but its README and provenance, by its path in NFC, as its - content hash, ``+x`` after it when it's executable.""" - tops = [root] if within is None else [root / part for part in within] +def written_files(root: Path) -> dict[str, str]: + """Each file under ``root`` but its README and provenance, by its path in NFC, as its ``file_digest``.""" files = {} - for path in sorted({path for top in tops for path in ([top] if top.is_file() else top.rglob("*"))}): + for path in sorted(root.rglob("*")): relative = unicodedata.normalize("NFC", path.relative_to(root).as_posix()) if path.is_file() and relative not in ("README.md", PROVENANCE): - files[relative] = file_sha256(path) + ("+x" if path.stat().st_mode & 0o111 else "") + files[relative] = file_digest(path) return files +def file_digest(path: Path) -> str: + """``path``'s content hash, ``+x`` after it when it's executable, as an import records each file it wrote.""" + return file_sha256(path) + ("+x" if path.stat().st_mode & 0o111 else "") + + def readme(description: str, imported: BundleImport) -> str: """The bundle's README: its description, where it came from, how to run and share it, what it holds, then the import's report.""" diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 7d987eb24..1655f1284 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -39,13 +39,14 @@ from agent_env.store.routing import configured_store, namespace_routing from agent_env.task_step.registry import get_task_step_registry -from ._import_report import written_files +from ._import_report import file_digest from ._imported import Imported, as_imported, grafted, read_imported, source_of, where from ._secrets import credentials_in +from .authoring import entry_files from .installed import installed_bundles from .ledger import Check, Ledger from .materialize import Materialization, Materialized, materialize -from .parse import Bundle, BundleEntry, BundleError, BundleKind, parse_bundle +from .parse import CONFIG_FILES, Bundle, BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle, plan_entities from .run import _label from .resolve import BundleImage, Reference, ResolvedBundle, ResolvedEntry, resolve_bundle @@ -239,11 +240,24 @@ def _held_as_imported(plan: Plan, imported: Imported | None) -> frozenset[str]: if imported is None: return frozenset() writes = [write for write in plan.writes if source_of(imported, plan, write) is not None] - root = plan.bundle.bundle.root - now = written_files(root, {where(root, write.source.entry) for write in writes}) + now = {} + for entry in {write.source.entry for write in writes}: + now |= _files_of(plan.bundle.bundle, entry) return frozenset(write.id for write in writes if as_imported(imported, plan, write, now)) +def _files_of(bundle: Bundle, entry: BundleEntry) -> dict[str, str]: + """The files a write of ``entry`` reads, as its writer walks them, by their paths in the bundle, as their + ``file_digest``: a task's or eval's file, or every file of its folder and its toml.""" + folder = where(bundle.root, entry) + if entry.kind in (BundleKind.TASK, BundleKind.EVAL): + return {folder: file_digest(entry.path)} + files = entry_files(bundle, entry, empty_ok=True) + if entry.kind in CONFIG_FILES and (toml := entry.path / CONFIG_FILES[entry.kind]).is_file(): + files[toml.name] = toml + return {f"{folder}/{key}": file_digest(path) for key, path in files.items()} + + def _sources(plan: Plan, ids: Mapping[str, str], imported: Imported | None, held: frozenset[str]) -> tuple[dict[tuple[str, str], int], set[tuple[str, str]]]: """For each write of ``plan``, under its ``@local`` ids, that the folder was imported from, by (store, registry id), @@ -420,8 +434,11 @@ def _as_imported(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Ch if stored is None or stored < self.sources[key]: return None digest = self.digest(write, needs) + row = self._latest(*key) + recorded = row is not None and digest is not None and (row["bundle"], row["version"], row.get("digest")) == ( + self._bundle(write), self.sources[key], digest.value) return Check(write, digest, self.sources[key], ("as it was imported",), stored, needs, - adopted=digest is not None) + adopted=digest is not None and not recorded) def _bundle(self, write: Write) -> str: """The bundle's identity, or for an entity an installed bundle comes with, that bundle's, which every bundle diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index ccfc721c5..b31bb7faa 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -74,7 +74,9 @@ def test_a_folder_registered_as_it_was_imported_writes_nothing_and_keeps_each_ve rows = get_config().get_document_store().query(REGISTRATIONS, Filter.of(status="done")) assert {(row["id"], row["version"]) for row in rows} == {("crm-image", 1), ("crm-env", 1), ("crm-data", 1), ("triage", 1)} + (imported / "envs/crm-env/.DS_Store").write_bytes(b"left by Finder") assert _written(register_bundle(imported)) == _written(registered) + assert len(get_config().get_document_store().query(REGISTRATIONS, Filter())) == len(rows) def test_an_edit_rewrites_only_what_it_reaches_and_gives_back_what_the_import_left_out(imported): @@ -351,3 +353,18 @@ def test_a_skill_the_folder_gives_a_source_of_its_own_keeps_it(local_stores, tmp skill = Task.get("skilled", 2).to_dict()["steps"][0]["skills"][0] assert {key: skill.get(key) for key in ("object_url", "s3_url", "body")} == { "object_url": source.get("object_url"), "s3_url": source.get("object_url"), "body": source.get("body")} + + +def test_a_link_into_an_imported_image_s_folder_is_an_edit_to_its_image(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + _pushed(monkeypatch) + files = {"Dockerfile": "FROM python:3.12-slim\nCOPY . /app\n", "app.py": "print(1)\n"} + MCPServerEnv.put(id="crm-env", docker_image_artifact=context_image(tmp_path, "crm-image", files), + environment_name="crm") + Task.put(id="one", steps=[build_step({"id": "env", "type": "deploy_env", "env_id": "crm-env"})]) + folder = import_task("one", tmp_path / "one").root + (folder / "envs/crm-env/tasks").symlink_to("../../tasks", target_is_directory=True) + + registered = register_bundle(folder) + + assert _written(registered)["crm-image"] == (2, False) From 3529897c084eb03d5fd80ad3dfb66341dc73278c Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 27 Oct 2026 23:42:44 -0700 Subject: [PATCH 17/22] fix(bundle): restore a masked value in a reordered list from the item it was masked from While a list is as the import left it, each item takes back its own position's value. Once it's edited, an item takes back the value of the one stored item whose scrubbed form it equals, so reordered downloads keep their own credentials, and an item that matches none, or several, keeps what it holds. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_imported.py | 24 ++++++++++++++++++------ tst/unit/bundle/register_imports_test.py | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index a20c25c64..57564b919 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -233,18 +233,30 @@ def _unscrubbed(value: object, stored: object, scrubbed: object, path: str = "") else: given += _unscrubbed(value[key], item, scrubbed[key], at) elif isinstance(value, list) and isinstance(stored, list) and isinstance(scrubbed, list) and ( - len(value) == len(stored) == len(scrubbed)): - for index, item in enumerate(stored): + len(stored) == len(scrubbed)): + unchanged = value == scrubbed + for index, item in enumerate(value): + if (source := _counterpart(value, index, scrubbed, unchanged)) is None: + continue at = f"{path}[{index}]" - if isinstance(item, str): - if value[index] == scrubbed[index] != item: - value[index] = item + if isinstance(stored[source], str): + if item == scrubbed[source] != stored[source]: + value[index] = stored[source] given.append(at) else: - given += _unscrubbed(value[index], item, scrubbed[index], at) + given += _unscrubbed(item, stored[source], scrubbed[source], at) return given +def _counterpart(value: list, index: int, scrubbed: list, unchanged: bool) -> int | None: + """The index of the stored item ``value[index]`` was scrubbed from: its own while the list is as scrubbed, else + the one scrubbed item it equals, when only one does, so a reordered list restores each item from its own.""" + if unchanged: + return index + same = [other for other, item in enumerate(scrubbed) if item == value[index]] + return same[0] if len(same) == 1 else None + + def _graft(step: dict, keys: list[str], value: object) -> bool: """Set ``keys`` in ``step`` to ``value`` when it's unset there; whether it was.""" holder = step diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index b31bb7faa..ea4153efa 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -368,3 +368,24 @@ def test_a_link_into_an_imported_image_s_folder_is_an_edit_to_its_image(local_st registered = register_bundle(folder) assert _written(registered)["crm-image"] == (2, False) + + +def test_reordered_downloads_each_take_back_their_own_credentials(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + # Not credentials: two users' URLs for one file, which the import masks to the same text, joined in at run time. + first, second = ("https://" + user + ":" + "not-a-token@data.example.com/rows.csv" for user in ("ann", "bob")) + urls = [{"url": first, "filename": "ann.csv"}, {"url": second, "filename": "bob.csv"}] + Task.put(id="fetch", steps=[build_step({"id": "box", "type": "deploy_sandbox", "sandbox_name": "box", + "sandbox_mode": "vm"}), + build_step({"id": "fetch", "type": "load_artifact", "sandbox_name": "box", + "urls": urls, "destination_path": "/data"})]) + folder = import_task("fetch", tmp_path / "fetch", include_scopes={FieldScope.USER}).root + task_file = folder / "tasks/fetch.json" + steps = json.loads(task_file.read_text()) + assert steps[1]["urls"][0]["url"] == steps[1]["urls"][1]["url"] != first + steps[1]["urls"].reverse() + task_file.write_text(json.dumps(steps)) + + register_bundle(folder) + + assert Task.get("fetch", 2).to_dict()["steps"][1]["urls"] == list(reversed(urls)) From 584b8dce149ad86a0f9e8d77ecc16e404a331c78 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 08:07:45 -0700 Subject: [PATCH 18/22] fix(bundle): give back a masked value to an edited item that holds it, and refuse one that can't be given back - An edited list item that equals no stored item matches the one whose masked values it still holds, so a renamed download takes back its URL. - A field still holding what the import masked after everything is given back refuses the registration, naming it, rather than writing the masked text. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_imported.py | 71 +++++++++++++++++++----- src/agent_env/bundle/register.py | 4 +- tst/unit/bundle/register_imports_test.py | 47 +++++++++++++--- 3 files changed, 101 insertions(+), 21 deletions(-) diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index 57564b919..14820bbf7 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -107,14 +107,15 @@ def as_imported(imported: Imported, plan: Plan, write: Write, now: Mapping[str, return then == current -def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]]]: +def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]], list[str]]: """``bundle`` with each imported task's steps given back, from the stored version each was imported from, matched by step id and type, what the import left out where the folder doesn't set it: each user-scoped field, but a reference's pin where the folder names something else; each list item's, from the stored item it was written from; and each env var and credential the import's scrub took, where the folder holds what the scrub left. And, for each - task's file, the paths in its steps that took one.""" + task's file, the paths in its steps that took one; and a problem for each field still holding what the scrub left, + which the folder changed too much to give back.""" registry = get_task_step_registry() - entries, taken = [], {} + entries, taken, problems = [], {}, [] for entry in bundle.entries: source = imported.entities.get(where(bundle.root, entry)) if entry.kind is BundleKind.TASK else None try: @@ -131,14 +132,18 @@ def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[ cls = registry.get(step.get("type")) if origin is None or cls is None or origin.get("type") != step.get("type"): continue - for path in _given_back(step, origin, cls): + given, left = _given_back(step, origin, cls) + for path in given: taken.setdefault(entry.path, set()).add(f"[{index}].{path}") + problems += [f"{where(bundle.root, entry)}: step {step.get('id')!r}: {path} still holds what the import " + "masked, and the folder changed what it was masked from; set its real value" for path in left] entries.append(replace(entry, config=steps)) - return replace(bundle, entries=tuple(entries)), taken + return replace(bundle, entries=tuple(entries)), taken, problems -def _given_back(step: dict, origin: dict, cls: type) -> list[str]: - """Give ``step`` back what the import left out of ``origin``, in place; the path of each field it took.""" +def _given_back(step: dict, origin: dict, cls: type) -> tuple[list[str], list[str]]: + """Give ``step`` back what the import left out of ``origin``, in place; the path of each field it took, and of each + still holding what the scrub left.""" user = [path for path, scope in cls.field_scopes.items() if scope is FieldScope.USER] repinned = _repinned(step, origin, cls) given, lists = [], {} @@ -157,7 +162,9 @@ def _given_back(step: dict, origin: dict, cls: type) -> list[str]: given += _given_back_items(step, origin, at, fields) scrubbed = copy.deepcopy(origin) scrub(scrubbed) - return given + _unscrubbed(step, origin, scrubbed) + given += _unscrubbed(step, origin, scrubbed) + masked = {text for _, text in _masks(origin, scrubbed)} + return given, [path for path, text in _texts(step) if text in masked] def _repinned(step: dict, origin: dict, cls: type) -> set[str]: @@ -236,7 +243,7 @@ def _unscrubbed(value: object, stored: object, scrubbed: object, path: str = "") len(stored) == len(scrubbed)): unchanged = value == scrubbed for index, item in enumerate(value): - if (source := _counterpart(value, index, scrubbed, unchanged)) is None: + if (source := _counterpart(item, index, stored, scrubbed, unchanged)) is None: continue at = f"{path}[{index}]" if isinstance(stored[source], str): @@ -248,15 +255,53 @@ def _unscrubbed(value: object, stored: object, scrubbed: object, path: str = "") return given -def _counterpart(value: list, index: int, scrubbed: list, unchanged: bool) -> int | None: - """The index of the stored item ``value[index]`` was scrubbed from: its own while the list is as scrubbed, else - the one scrubbed item it equals, when only one does, so a reordered list restores each item from its own.""" +def _counterpart(item: object, index: int, stored: list, scrubbed: list, unchanged: bool) -> int | None: + """The index of the stored item ``item``, at ``index``, was scrubbed from: its own while the list is as scrubbed; + else the one scrubbed item it equals, or failing that the one whose masked values it holds, when only one does, so + a reordered or edited list restores each item from its own.""" if unchanged: return index - same = [other for other, item in enumerate(scrubbed) if item == value[index]] + same = [other for other, one in enumerate(scrubbed) if one == item] + if not same: + same = [other for other, one in enumerate(scrubbed) if (masks := _masks(stored[other], one)) + and all(_at_keys(item, keys) == text for keys, text in masks)] return same[0] if len(same) == 1 else None +def _masks(stored: object, scrubbed: object, keys: tuple = ()) -> list[tuple[tuple, str]]: + """Each string scrubbing ``stored`` masked: the keys to it, and the text the scrub left.""" + if isinstance(stored, str): + return [(keys, scrubbed)] if isinstance(scrubbed, str) and scrubbed != stored else [] + if isinstance(stored, dict) and isinstance(scrubbed, dict): + return [mask for key in stored if key in scrubbed for mask in _masks(stored[key], scrubbed[key], (*keys, key))] + if isinstance(stored, list) and isinstance(scrubbed, list) and len(stored) == len(scrubbed): + return [mask for index, (one, other) in enumerate(zip(stored, scrubbed)) + for mask in _masks(one, other, (*keys, index))] + return [] + + +def _at_keys(value: object, keys: tuple) -> object: + for key in keys: + if isinstance(value, dict) and isinstance(key, str): + value = value.get(key) + elif isinstance(value, list) and isinstance(key, int) and key < len(value): + value = value[key] + else: + return None + return value + + +def _texts(value: object, path: str = "") -> list[tuple[str, str]]: + """Each string in ``value``, with its path.""" + if isinstance(value, str): + return [(path, value)] + if isinstance(value, dict): + return [text for key, item in value.items() for text in _texts(item, f"{path}.{key}" if path else str(key))] + if isinstance(value, list): + return [text for index, item in enumerate(value) for text in _texts(item, f"{path}[{index}]")] + return [] + + def _graft(step: dict, keys: list[str], value: object) -> bool: """Set ``keys`` in ``step`` to ``value`` when it's unset there; whether it was.""" holder = step diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 1655f1284..b479114c1 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -95,7 +95,9 @@ def register_bundle( imported, why = read_imported(bundle.root) if why is not None: say(f" {why}") - bundle, grafts = grafted(bundle, imported) if imported is not None else (bundle, {}) + bundle, grafts, unrestored = grafted(bundle, imported) if imported is not None else (bundle, {}, []) + if unrestored: + raise BundleError(unrestored) resolved = resolve_bundle(bundle) plan = _plan(resolved, tasks, evals) held = _held_as_imported(plan, imported) diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index ea4153efa..9a0fe0c9f 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -370,22 +370,55 @@ def test_a_link_into_an_imported_image_s_folder_is_an_edit_to_its_image(local_st assert _written(registered)["crm-image"] == (2, False) -def test_reordered_downloads_each_take_back_their_own_credentials(local_stores, tmp_path, monkeypatch): +def _downloads(tmp_path, monkeypatch, names): + """The stored task fetch, downloading one file for each of ``names`` from that user's URL, which the import masks + to the same text, imported with its user-scoped fields; the folder, and the urls the task stores.""" monkeypatch.setenv("HOME", str(tmp_path)) - # Not credentials: two users' URLs for one file, which the import masks to the same text, joined in at run time. - first, second = ("https://" + user + ":" + "not-a-token@data.example.com/rows.csv" for user in ("ann", "bob")) - urls = [{"url": first, "filename": "ann.csv"}, {"url": second, "filename": "bob.csv"}] + # Not credentials: users' URLs for one file, joined in at run time. + urls = [{"url": "https://" + name + ":" + "not-a-token@data.example.com/rows.csv", "filename": f"{name}.csv"} + for name in names] Task.put(id="fetch", steps=[build_step({"id": "box", "type": "deploy_sandbox", "sandbox_name": "box", "sandbox_mode": "vm"}), build_step({"id": "fetch", "type": "load_artifact", "sandbox_name": "box", "urls": urls, "destination_path": "/data"})]) - folder = import_task("fetch", tmp_path / "fetch", include_scopes={FieldScope.USER}).root + return import_task("fetch", tmp_path / "fetch", include_scopes={FieldScope.USER}).root, urls + + +def _edit_urls(folder, edit): task_file = folder / "tasks/fetch.json" steps = json.loads(task_file.read_text()) - assert steps[1]["urls"][0]["url"] == steps[1]["urls"][1]["url"] != first - steps[1]["urls"].reverse() + edit(steps[1]["urls"]) task_file.write_text(json.dumps(steps)) + +def test_reordered_downloads_each_take_back_their_own_credentials(local_stores, tmp_path, monkeypatch): + folder, urls = _downloads(tmp_path, monkeypatch, ["ann", "bob"]) + _edit_urls(folder, list.reverse) + register_bundle(folder) assert Task.get("fetch", 2).to_dict()["steps"][1]["urls"] == list(reversed(urls)) + + +def test_a_renamed_download_takes_back_its_credentials(local_stores, tmp_path, monkeypatch): + folder, urls = _downloads(tmp_path, monkeypatch, ["ann"]) + _edit_urls(folder, lambda entries: entries[0].update(filename="rows.csv")) + + register_bundle(folder) + + assert Task.get("fetch", 2).to_dict()["steps"][1]["urls"] == [{"url": urls[0]["url"], "filename": "rows.csv"}] + + +def test_a_masked_value_that_cant_be_told_from_another_is_refused_rather_than_written(local_stores, tmp_path, + monkeypatch): + folder, _ = _downloads(tmp_path, monkeypatch, ["ann", "bob"]) + _edit_urls(folder, lambda entries: [entry.update(filename=f"renamed-{index}.csv") + for index, entry in enumerate(entries)]) + + with pytest.raises(BundleError) as refused: + register_bundle(folder) + + assert refused.value.problems == tuple( + f"tasks/fetch.json: step 'fetch': urls[{index}].url still holds what the import masked, and the folder " + "changed what it was masked from; set its real value" for index in (0, 1)) + assert Task.get("fetch").version == 1 From f28c646ae692fa4ffd390b33b5afb8656e2dad03 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 08:28:10 -0700 Subject: [PATCH 19/22] fix(bundle): refuse a masked value only in a task the registration writes A task the selection doesn't reach is never written, so what it still holds masked refuses nothing. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_imported.py | 11 ++++++----- src/agent_env/bundle/register.py | 7 ++++--- tst/unit/bundle/register_imports_test.py | 12 ++++++++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index 14820bbf7..fc5aa43e7 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -107,15 +107,15 @@ def as_imported(imported: Imported, plan: Plan, write: Write, now: Mapping[str, return then == current -def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]], list[str]]: +def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]], dict[Path, list[str]]]: """``bundle`` with each imported task's steps given back, from the stored version each was imported from, matched by step id and type, what the import left out where the folder doesn't set it: each user-scoped field, but a reference's pin where the folder names something else; each list item's, from the stored item it was written from; and each env var and credential the import's scrub took, where the folder holds what the scrub left. And, for each - task's file, the paths in its steps that took one; and a problem for each field still holding what the scrub left, + task's file, the paths in its steps that took one, and a problem for each field still holding what the scrub left, which the folder changed too much to give back.""" registry = get_task_step_registry() - entries, taken, problems = [], {}, [] + entries, taken, problems = [], {}, {} for entry in bundle.entries: source = imported.entities.get(where(bundle.root, entry)) if entry.kind is BundleKind.TASK else None try: @@ -135,8 +135,9 @@ def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[ given, left = _given_back(step, origin, cls) for path in given: taken.setdefault(entry.path, set()).add(f"[{index}].{path}") - problems += [f"{where(bundle.root, entry)}: step {step.get('id')!r}: {path} still holds what the import " - "masked, and the folder changed what it was masked from; set its real value" for path in left] + problems.setdefault(entry.path, []).extend( + f"{where(bundle.root, entry)}: step {step.get('id')!r}: {path} still holds what the import masked, and " + "the folder changed what it was masked from; set its real value" for path in left) entries.append(replace(entry, config=steps)) return replace(bundle, entries=tuple(entries)), taken, problems diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index b479114c1..7aa798ea2 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -95,11 +95,12 @@ def register_bundle( imported, why = read_imported(bundle.root) if why is not None: say(f" {why}") - bundle, grafts, unrestored = grafted(bundle, imported) if imported is not None else (bundle, {}, []) - if unrestored: - raise BundleError(unrestored) + bundle, grafts, unrestored = grafted(bundle, imported) if imported is not None else (bundle, {}, {}) resolved = resolve_bundle(bundle) plan = _plan(resolved, tasks, evals) + if problems := [problem for write in plan.writes if write.kind is BundleKind.TASK + for problem in unrestored.get(write.source.entry.path, ())]: + raise BundleError(problems) held = _held_as_imported(plan, imported) ids, problems = registry_ids(plan, _prefix(bundle, prefix), imported, held) if problems: diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index 9a0fe0c9f..d480af788 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -422,3 +422,15 @@ def test_a_masked_value_that_cant_be_told_from_another_is_refused_rather_than_wr f"tasks/fetch.json: step 'fetch': urls[{index}].url still holds what the import masked, and the folder " "changed what it was masked from; set its real value" for index in (0, 1)) assert Task.get("fetch").version == 1 + + +def test_a_masked_value_in_a_task_not_registered_refuses_nothing(local_stores, tmp_path, monkeypatch): + folder, _ = _downloads(tmp_path, monkeypatch, ["ann", "bob"]) + _edit_urls(folder, lambda entries: [entry.update(filename=f"renamed-{index}.csv") + for index, entry in enumerate(entries)]) + (folder / "tasks/box.json").write_text(json.dumps([{"id": "box", "type": "deploy_sandbox", "sandbox_name": "box", + "sandbox_mode": "vm"}])) + + registered = register_bundle(folder, tasks=["box"]) + + assert _written(registered) == {"fetch-box": (1, False)} From 530bb76c8d47ec1874d57eee3a2102fa68e5858a Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 09:35:14 -0700 Subject: [PATCH 20/22] refactor(bundle): drop the ownership check and --adopt from bundle register An id the store holds already now takes the bundle's next version, whoever wrote the store's latest. Two bundles taking one id is unlikely, and the check cost a refusal, its recheck at each write, and --adopt to get past it. The strict version each write lands on stays, so two registrations racing for one id still can't both write it. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/register.py | 73 ++++++-------------------------- src/agent_env/cli/bundle.py | 14 +++--- tst/unit/bundle/register_test.py | 66 ++++------------------------- 3 files changed, 27 insertions(+), 126 deletions(-) diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 2cdeb4672..cb2477bde 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -9,12 +9,11 @@ under a tag of its sources, with its build context beside it; one published for its sources is registered as published. Nothing is written when any problem is found before the first write: an id the rules can't give, two writes to one store -given one id, an id the store holds that this bundle didn't register (unless adopted), an ``@local`` id left in what -would be written, a store reference the store doesn't hold, or, for a store that isn't this machine's, a credential -written out or a URL that only reaches this machine. A dry run checks the same and writes, builds and pushes nothing. -Each write goes through a ledger kept in the target store, so registering again writes only what changed. Two -registrations racing for one id can't both write a version of it: the loser fails, and running it again reuses what the -winner wrote. +given one id, an ``@local`` id left in what would be written, a store reference the store doesn't hold, or, for a store +that isn't this machine's, a credential written out or a URL that only reaches this machine. A dry run checks the same +and writes, builds and pushes nothing. Each write goes through a ledger kept in the target store, so registering again +writes only what changed. Two registrations racing for one id can't both write a version of it: the loser fails, and +running it again reuses what the winner wrote. """ from __future__ import annotations @@ -31,7 +30,6 @@ from agent_env.config import get_config from agent_env.entity.refs import RefRole, ref_sites -from agent_env.store import Filter from agent_env.plugins._bundles import AGENT_ALIASES from agent_env.store.document_store import DocumentStore, LocalSqliteDocumentStore from agent_env.store.ids import LOCAL_PREFIX, is_local_id @@ -41,7 +39,7 @@ from ._secrets import credentials_in from .installed import installed_bundles -from .ledger import Check, Ledger +from .ledger import Ledger from .materialize import Materialization, Materialized, materialize from .parse import Bundle, BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle, plan_entities @@ -74,15 +72,13 @@ def register_bundle( tasks: Sequence[str] = (), evals: Sequence[str] = (), prefix: str | None = None, - adopt: Sequence[str] = (), dry_run: bool = False, on_progress: Callable[[str], None] | None = None, ) -> Registration: """Register the bundle at ``root``: ``tasks`` and ``evals`` by name, or with neither every task and eval it has, or every entity when it has none, with what they reference. ``prefix`` opens each entry's registry id, the folder's - name and a dash unless given. ``adopt`` names ids the store holds that this bundle may write new versions of though - it didn't register them. ``on_progress`` is given a line for each write, as it's checked or written, and before - each image is built. + name and a dash unless given. An id the store holds already takes a new version, whoever wrote it. ``on_progress`` + is given a line for each write, as it's checked or written, and before each image is built. It turns namespace routing on while it runs. Raises BundleError listing every problem found before anything is written; a write that fails stops it, and every earlier one stays, in the ledger, for the next run to reuse.""" @@ -97,8 +93,8 @@ def register_bundle( target = configured_store(get_config().get_document_store()) shared = shared_store(target) identity, commit, dirty = bundle_identity(bundle.root) - ledger = RegistrationLedger(target, registered, identity, commit, dirty, frozenset(adopt)) - problems = [*_unowned(registered, ledger), *_unregistrable(registered, shared)] + ledger = RegistrationLedger(target, registered, identity, commit, dirty) + problems = list(_unregistrable(registered, shared)) if problems: raise BundleError(problems) say = on_progress or (lambda line: None) @@ -297,46 +293,9 @@ class RegistrationLedger(Ledger): strict = True pushes_images = True - def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None, - adopt: frozenset[str] = frozenset()): + def __init__(self, store: DocumentStore, plan: Plan, identity: str, commit: str | None, dirty: bool | None): super().__init__(store, plan) - self.identity, self.commit, self.dirty, self.adopted_ids = identity, commit, dirty, adopt - - def check(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Check: - """``Ledger.check``, refusing a write whose id the store holds that this bundle didn't register, checked again - as it's about to be written, since another bundle can write the id after the check before any write.""" - check = super().check(write, needs) - if (problem := self.unowned(write, check.stored)) is not None: - raise BundleError([problem]) - return check - - def unowned(self, write: Write, stored: int | None) -> str | None: - """Why this bundle may not write ``write``'s id, which the store holds at ``stored``, or None when it may: the - store doesn't hold it, ``adopt`` names it, an interrupted registration of this bundle wrote its latest version, - or that's the version this bundle last registered. One the store has written anew since is refused, so no one - else's version is written over unseen.""" - if stored is None or write.id in self.adopted_ids: - return None - pending = self._find(self.collection, Filter.of(store=write.kind.store, id=write.id, bundle=self._bundle(write), - status="pending")) - if pending is not None and pending.get("version") == stored: - return None - where = self._plan.path(write.source.entry) - row = self._latest(write.kind.store, write.id) - if row is not None and row.get("bundle") == self._bundle(write): - return None if row.get("version") == stored else self._moved(where, write.id, stored, row["version"], - "this bundle last registered") - by = f"registered from {row['bundle']}" if row is not None else "not registered from a bundle" - return (f"{where}: {write.id!r} is in the store already, at v{stored}, {by}; pass --adopt {write.id} to write a " - "new version of it, or --prefix to register this bundle under other ids") - - @staticmethod - def _moved(where: str, entity_id: str, stored: int, own: int, whose: str) -> str: - return (f"{where}: {entity_id!r} is at v{stored} in the store, newer than the v{own} {whose}; pass --adopt " - f"{entity_id} to write over it") - - def stored_version(self, write: Write) -> int | None: - return self._stored_version(write) + self.identity, self.commit, self.dirty = identity, commit, dirty def _bundle(self, write: Write) -> str: """The bundle's identity, or for an entity an installed bundle comes with, that bundle's, which every bundle @@ -380,14 +339,6 @@ def _remote_name(remote: str) -> str: return f"{host}/{path.strip('/').removesuffix('.git')}" -def _unowned(plan: Plan, ledger: RegistrationLedger) -> Iterator[str]: - """A problem for each write this bundle may not make (``RegistrationLedger.unowned``), every one found before any - write.""" - for write in plan.writes: - if (problem := ledger.unowned(write, ledger.stored_version(write))) is not None: - yield problem - - def _unregistrable(plan: Plan, shared: bool) -> Iterator[str]: """A problem for each ``@local`` id left in what the plan writes and, for a ``shared`` store, each credential written out and each URL that reaches only this machine.""" diff --git a/src/agent_env/cli/bundle.py b/src/agent_env/cli/bundle.py index ec4998c26..79e6adaad 100644 --- a/src/agent_env/cli/bundle.py +++ b/src/agent_env/cli/bundle.py @@ -71,12 +71,10 @@ def _report(imported: BundleImport, folder: Path) -> None: @click.option("--eval", "evals", multiple=True, help="Register this eval, by name, and the tasks it names. Repeatable.") @click.option("--prefix", default=None, help="What each entry's registry id opens with; by default the folder's name and a dash.") -@click.option("--adopt", multiple=True, - help="An id the store holds that this bundle didn't register, to write a new version of. Repeatable.") @click.option("--dry-run", is_flag=True, help="Check everything and say what would be written; write nothing.") @click.option("--yes", is_flag=True, help="Write to a store other than this machine's own without asking.") def register(folder: Path, tasks: tuple[str, ...], evals: tuple[str, ...], prefix: str | None, - adopt: tuple[str, ...], dry_run: bool, yes: bool): + dry_run: bool, yes: bool): """Write the bundle at FOLDER into the configured stores under registry ids, so whatever runs the stores' tasks runs what the folder runs. @@ -87,11 +85,11 @@ def register(folder: Path, tasks: tuple[str, ...], evals: tuple[str, ...], prefi image the bundle builds is built here for linux/amd64 with Docker and pushed to the configured image store, with its build context beside it. - It checks everything first and says what it would write: an id the store holds that this bundle didn't register, - an @local id left in what it would write, or, for a store other than this machine's, a credential written out or a - URL only this machine reaches, refuses the whole registration. A store other than this machine's is written only - after you confirm, or with --yes. Registering again writes only what changed since.""" - options = {"tasks": tasks, "evals": evals, "prefix": prefix, "adopt": adopt} + It checks everything first and says what it would write: an @local id left in what it would write or, for a store + other than this machine's, a credential written out or a URL only this machine reaches refuses the whole + registration. An id the store holds already takes a new version, whoever wrote it. A store other than this + machine's is written only after you confirm, or with --yes. Registering again writes only what changed since.""" + options = {"tasks": tasks, "evals": evals, "prefix": prefix} click.echo(f"Registering {folder} into:") for line in describe_target(): click.echo(f" {line}") diff --git a/tst/unit/bundle/register_test.py b/tst/unit/bundle/register_test.py index 136647b27..702f7b806 100644 --- a/tst/unit/bundle/register_test.py +++ b/tst/unit/bundle/register_test.py @@ -163,29 +163,20 @@ def test_a_remote_is_named_by_its_host_and_path_alone(remote, name): assert _remote_name(remote) == name -def test_an_id_the_store_holds_that_the_bundle_didnt_register_is_refused_unless_adopted(home): +def test_an_id_the_store_holds_takes_the_bundle_s_next_version_whoever_wrote_it(home): (home / "seed.txt").write_text("seed") FileArtifact.put("triage-docs", description="someone else's", file_path=str(home / "seed.txt")) root = layout(home / "triage", TRIAGE) - with pytest.raises(BundleError) as refused: - register_bundle(root) - adopted = register_bundle(root, adopt=["triage-docs"]) - - assert refused.value.problems == ( - "artifacts/docs: 'triage-docs' is in the store already, at v1, not registered from a bundle; pass --adopt " - "triage-docs to write a new version of it, or --prefix to register this bundle under other ids",) - assert {done.write.id: done.version for done in adopted.materialization.writes}["triage-docs"] == 2 - - -def test_another_folder_of_the_same_name_is_refused_the_ids_this_one_registered(home): - register_bundle(layout(home / "a" / "triage", TRIAGE)) - - with pytest.raises(BundleError) as refused: - register_bundle(layout(home / "b" / "triage", TRIAGE)) + first = register_bundle(root) + FileArtifact.put("triage-docs", description="someone else's again", file_path=str(home / "seed.txt")) + (root / "artifacts/docs/a.md").write_text("changed\n") + second = register_bundle(root) + elsewhere = register_bundle(layout(home / "b" / "triage", TRIAGE)) - assert len(refused.value.problems) == 3 - assert all(f"registered from folder:{home / 'a' / 'triage'}" in problem for problem in refused.value.problems) + assert {done.write.id: done.version for done in first.materialization.writes}["triage-docs"] == 2 + assert {done.write.id: done.version for done in second.materialization.writes}["triage-docs"] == 4 + assert {done.write.id: done.version for done in elsewhere.materialization.writes}["triage-docs"] == 5 def test_a_prefix_registers_under_other_ids(home): @@ -524,42 +515,3 @@ def dies_after_writing_docs(self, write, needs): assert {done.write.id: (done.version, done.reused) for done in resumed.materialization.writes} == { "triage-docs": (1, True), "triage-greeting": (1, False), "triage-t": (1, False)} - - -def test_an_id_another_bundle_registers_after_the_check_is_refused_before_it_is_written(home, monkeypatch): - root = layout(home / "triage", TRIAGE) - unowned = register_module._unowned - - def then_another_bundle_writes(plan, ledger): - problems = list(unowned(plan, ledger)) - (home / "theirs.txt").write_text("theirs") - FileArtifact.put("triage-docs", description="theirs", file_path=str(home / "theirs.txt")) - get_config().get_document_store().insert(REGISTRATIONS, { - "store": "artifact", "id": "triage-docs", "bundle": "folder:/elsewhere/triage", "status": "done", - "version": 1, "scheme": 2, "digest": None, "inputs": None}) - return iter(problems) - - monkeypatch.setattr(register_module, "_unowned", then_another_bundle_writes) - - with pytest.raises(BundleError) as refused: - register_bundle(root) - - assert "'triage-docs' is in the store already, at v1, registered from folder:/elsewhere/triage" in \ - refused.value.problems[0] - assert Artifact.get("triage-docs").description == "theirs" - - -def test_a_version_someone_else_wrote_since_is_not_written_over_unless_adopted(home): - root = layout(home / "triage", TRIAGE) - register_bundle(root) - (home / "seed.txt").write_text("seed") - FileArtifact.put("triage-docs", description="someone else's", file_path=str(home / "seed.txt")) - (root / "artifacts/docs/a.md").write_text("changed\n") - - with pytest.raises(BundleError) as refused: - register_bundle(root) - adopted = register_bundle(root, adopt=["triage-docs"]) - - assert refused.value.problems == ("artifacts/docs: 'triage-docs' is at v2 in the store, newer than the v1 this " - "bundle last registered; pass --adopt triage-docs to write over it",) - assert {done.write.id: done.version for done in adopted.materialization.writes}["triage-docs"] == 3 From 4aee3642af277f443f6ee9af37be287616996c7f Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 11:16:13 -0700 Subject: [PATCH 21/22] fix(bundle): fingerprint where the document store is, and give an edited skill nothing back by its name - The store fingerprint resolves secret: references as well as env:, and hashes only the store's impl and the strings saying where it is: a URL's scheme, host and path, nothing a secret-named key holds, and no tuning option. Two stages whose configs hold the same text but whose secrets reach different databases no longer look like one store, and a teammate's different timeout no longer makes the folder register as a new bundle. - A list item matches its stored item only by being it apart from the fields the import left out, so an edited skill takes back no source by sharing a name, and registration says it names none. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_import_report.py | 33 ++++++++++++++++----- src/agent_env/bundle/_imported.py | 10 ++----- tst/unit/bundle/importer_entities_test.py | 36 ++++++++++++++++------- tst/unit/bundle/register_imports_test.py | 11 +++++-- 4 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index e611d9544..747c099db 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -11,11 +11,12 @@ from datetime import datetime, timezone from importlib.metadata import version as package_version from pathlib import Path +from typing import Any +from urllib.parse import urlsplit from agent_env.bundle._fs import file_sha256 -from agent_env.bundle._secrets import scrub from agent_env.config import get_config -from agent_env.config.loader import SECRET_REF_PREFIX, interpolate +from agent_env.config.describe import is_secret_key from agent_env.config.model import ENV_API_KEY, ENV_BASE_URL from agent_env.entity.refs import EntityKind from agent_env.store.image_store.oci_registry_credentials import registry_host_from_ref @@ -191,15 +192,31 @@ def provenance(source: dict, imported: BundleImport, root: Path) -> str: def store_fingerprint() -> str: - """A hash of the configured document store's section, with its ``env:`` references resolved and any credential - in it masked, which tells one store from another without naming where either is.""" - section = interpolate(get_config().trace_section("document").value, - secret_resolver=lambda key: SECRET_REF_PREFIX + key) - scrub(section) - text = json.dumps(section, sort_keys=True, separators=(",", ":"), default=str) + """A hash of where the configured document store is: its section's ``impl``, and the strings its ``config`` holds, + ``env:`` and ``secret:`` references resolved, a URL as its scheme, host and path alone, and nothing a secret-named + key holds. Credentials and tuning options don't tell one store from another, so neither changes it, and the hash + names neither where the store is nor how to reach it.""" + config = get_config() + section = config.trace_section("document").value + located = {"impl": str(section.get("impl")), "config": _location(config.resolve_reference(section.get("config", {})))} + text = json.dumps(located, sort_keys=True, separators=(",", ":")) return "sha256:" + hashlib.sha256(text.encode()).hexdigest() +def _location(value: Any) -> Any: + """What in ``value`` says where a store is: each string, a URL as its scheme, host and path, through dicts but the + items of secret-named keys, and lists; None for anything else.""" + if isinstance(value, dict): + return {key: located for key, item in value.items() + if not is_secret_key(str(key)) and (located := _location(item)) is not None} + if isinstance(value, list): + return [located for item in value if (located := _location(item)) is not None] + if isinstance(value, str): + parts = urlsplit(value) + return f"{parts.scheme}://{parts.netloc.rpartition('@')[2]}{parts.path}" if parts.scheme and parts.netloc else value + return None + + def written_files(root: Path) -> dict[str, str]: """Each file under ``root`` but its README and provenance, by its path in NFC, as its ``file_digest``.""" files = {} diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index fc5aa43e7..c8ba3074e 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -199,14 +199,10 @@ def _given_back_items(step: dict, origin: dict, at: str, fields: list[str]) -> l def _source_item(item: dict, stored: list, left_out: set[str]) -> dict | None: - """The stored item ``item`` was written from: the one it equals but for the ``left_out`` fields, else the one alone - in sharing its name, when ``item`` sets nothing that one doesn't.""" - candidates = [one for one in stored if isinstance(one, dict)] + """The stored item ``item`` was written from: the one it equals but for the ``left_out`` fields, when only one + does. An edited item matches none, so it takes back nothing.""" bare = _bare(item, left_out) - same = [one for one in candidates if _bare(one, left_out) == bare] - if not same and item.get("name") is not None: - same = [one for one in candidates - if one.get("name") == item["name"] and bare.keys() <= _bare(one, left_out).keys()] + same = [one for one in stored if isinstance(one, dict) and _bare(one, left_out) == bare] return same[0] if len(same) == 1 else None diff --git a/tst/unit/bundle/importer_entities_test.py b/tst/unit/bundle/importer_entities_test.py index 4a9fd71e7..dec45446f 100644 --- a/tst/unit/bundle/importer_entities_test.py +++ b/tst/unit/bundle/importer_entities_test.py @@ -313,24 +313,38 @@ def test_the_store_fingerprint_tells_one_configured_document_store_from_another( assert store_fingerprint() != here and store_fingerprint() == store_fingerprint() -def test_the_store_fingerprint_resolves_env_references_and_ignores_credentials(tmp_path, monkeypatch): +def test_the_store_fingerprint_is_where_the_store_is_whatever_its_credentials_and_tuning(tmp_path, monkeypatch): + """Two stages' configs can hold the same text and reach different stores through what their secrets resolve to.""" config = tmp_path / "fingerprint.toml" monkeypatch.setenv("AGENT_ENV_CONFIG", str(config)) monkeypatch.delenv("AGENT_ENV_DOCUMENT_STORE", raising=False) - - def fingerprint(path, uri): - config.write_text(f'[stores.document]\nimpl = "pkg:Store"\n[stores.document.config]\npath = "{path}"\n' - f'uri = "{uri}"\n') + monkeypatch.delenv("AGENT_ENV_SECRET_STORE", raising=False) + + def fingerprint(database, password, extra=""): + # Not a credential: the password a URL's user info holds in this test, joined in at run time. + uri = "mongodb://reader:" + password + "@db.example.com:27017/?retryWrites=true" + config.write_text(f'''[stores.document] +impl = "pkg:Store" +[stores.document.config] +uri = "secret:mongodb_uri" +database = "secret:mongodb_name" +{extra} +[stores.secret] +impl = "agent_env.store.secret_store:LocalSecretStore" +[stores.secret.config] +use_env = false +values = {{ mongodb_uri = "{uri}", mongodb_name = "{database}" }} +''') reset_config() return store_fingerprint() - monkeypatch.setenv("DOCUMENT_PATH", "/data/one.db") - one = fingerprint("env:DOCUMENT_PATH", "mongodb://reader:" + "pw-one@db.example.com/agent_env") - monkeypatch.setenv("DOCUMENT_PATH", "/data/two.db") - two = fingerprint("env:DOCUMENT_PATH", "mongodb://reader:" + "pw-one@db.example.com/agent_env") - other_password = fingerprint("env:DOCUMENT_PATH", "mongodb://reader:" + "pw-two@db.example.com/agent_env") + prod = fingerprint("agent_env", "pw-one") + dev = fingerprint("agent_env_dev", "pw-one") + other_password = fingerprint("agent_env", "pw-two") + tuned = fingerprint("agent_env", "pw-one", "connect_timeout_ms = 9000\npassword = \"not-a-password\"") - assert one != two and two == other_password + assert prod != dev + assert prod == other_password == tuned def _saved_image(path, url, files): diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index 252f6c1bc..e4e0a3c18 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -205,20 +205,25 @@ def test_an_edit_keeps_the_env_vars_the_import_dropped_and_they_are_no_credentia assert Task.get("triage", 2).to_dict()["steps"][2]["env_vars"] == {"API_KEY": "abc", "MODE": "fast"} -def test_an_imported_skill_takes_back_the_source_the_import_left_out(local_stores, tmp_path, monkeypatch): +def test_an_imported_skill_takes_back_the_source_the_import_left_out_unless_it_was_edited(local_stores, tmp_path, + monkeypatch): monkeypatch.setenv("HOME", str(tmp_path)) notes = {"name": "notes", "description": "how notes are kept", "object_url": "s3://their-bucket/skills/notes/"} Task.put(id="skilled", steps=[build_step({"id": "skills", "type": "add_skills", "agent_name": "default-agent", "skills": [notes]})]) folder = import_task("skilled", tmp_path / "skilled").root task_file = folder / "tasks/skilled.json" - task_file.write_text(task_file.read_text().replace("how notes are kept", "how notes are filed")) + task_file.write_text(task_file.read_text().replace('"default-agent"', '"helper"')) registered = register_bundle(folder) + task_file.write_text(task_file.read_text().replace("how notes are kept", "how notes are filed")) + with pytest.raises(BundleError) as refused: + register_bundle(folder) assert _written(registered)["skilled"] == (2, False) skill = Task.get("skilled", 2).to_dict()["steps"][0]["skills"][0] - assert (skill["description"], skill["object_url"]) == ("how notes are filed", notes["object_url"]) + assert (skill["description"], skill["object_url"]) == ("how notes are kept", notes["object_url"]) + assert "skill 'notes' names no source" in refused.value.problems[0] def test_envs_sharing_one_stored_image_register_as_imported_and_an_edit_to_one_copy_is_refused(local_stores, tmp_path, From e71ec777d6714ca9f318a51705e3cbfe8fb222a4 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 11:38:23 -0700 Subject: [PATCH 22/22] refactor(bundle): gather what an import record says of a plan into ImportedWrites - ImportedWrites, built once from the record, the plan and what restore gave back, holds each imported write's source, which writes the folder holds as imported, and what couldn't be given back, and answers the record's ids, the shared image copies check and the versions the ledger reads. register.py drops _given, _held_as_imported, _files_of and _sources, and passes it in place of five separate values. - Writes are told apart by store and @local id there, since a task and an env of one name share an @local id. - A record's id and version are a Source, and an image's are an ImportedImage with its Dockerfile, in place of positional tuples and two maps of one key. - registry_ids's ids the record gives are `recorded`, no longer `kept`, which elsewhere means the writes left as imported. - One string walk and one key-path lookup, in _values, in place of _strings, _texts, _at and _at_keys. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_imported.py | 186 ++++++++++++++++------- src/agent_env/bundle/_values.py | 30 ++++ src/agent_env/bundle/register.py | 134 ++++------------ tst/unit/bundle/register_imports_test.py | 5 +- 4 files changed, 197 insertions(+), 158 deletions(-) create mode 100644 src/agent_env/bundle/_values.py diff --git a/src/agent_env/bundle/_imported.py b/src/agent_env/bundle/_imported.py index c8ba3074e..8232e6268 100644 --- a/src/agent_env/bundle/_imported.py +++ b/src/agent_env/bundle/_imported.py @@ -7,9 +7,10 @@ import copy import json import unicodedata -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import dataclass, replace from pathlib import Path +from typing import NamedTuple from agent_env.entity.paths import field_sites from agent_env.entity.refs import ref_sites @@ -19,23 +20,38 @@ from agent_env.task.task import Task from agent_env.task_step.registry import get_task_step_registry -from ._import_report import PROVENANCE, PROVENANCE_SCHEMA, store_fingerprint +from ._import_report import PROVENANCE, PROVENANCE_SCHEMA, file_digest, store_fingerprint from ._secrets import scrub +from ._values import strings, value_at +from .authoring import entry_files from .parse import CONFIG_FILES, Bundle, BundleEntry, BundleKind from .plan import Plan, Write, env_folder_writer from .resolve import BuiltImage, BundleImage +class Source(NamedTuple): + """The store id and version a write was imported from.""" + + id: str + version: int + + +@dataclass(frozen=True) +class ImportedImage: + """An image the import wrote into a folder, under a toml key: built from the Dockerfile it names, or pulled.""" + + source: Source + dockerfile: str | None = None + + @dataclass(frozen=True) class Imported: - """What an import recorded: each entity folder, task file and eval file's store id and version, each image's by its - folder and toml key, built or pulled, with the Dockerfile a built one is built from, and each file as it wrote - it.""" + """What an import recorded: the stored id and version of each entity folder, task file and eval file, and of each + image a folder names, by its folder and toml key; and each file as it wrote it.""" - entities: Mapping[str, tuple[str, int]] - images: Mapping[tuple[str, str], tuple[str, int]] + entities: Mapping[str, Source] + images: Mapping[tuple[str, str], ImportedImage] files: Mapping[str, str] - dockerfiles: Mapping[tuple[str, str], str] def read_imported(root: Path) -> tuple[Imported | None, str | None]: @@ -52,14 +68,15 @@ def read_imported(root: Path) -> tuple[Imported | None, str | None]: return None, f"{PROVENANCE} is from an older agent-env, so the folder registers as a new bundle" if record.get("store") != store_fingerprint(): return None, "it was imported from another store, so its entries take new ids" - entities = {item["folder"]: (item["id"], item["version"]) for item in record.get("written", [])} - entities |= {item["file"]: (item["id"], item["version"]) for item in record.get("tasks", [])} + entities = {item["folder"]: Source(item["id"], item["version"]) for item in record.get("written", [])} + entities |= {item["file"]: Source(item["id"], item["version"]) for item in record.get("tasks", [])} if (stored_eval := record.get("eval")) is not None: - entities[stored_eval["file"]] = (stored_eval["id"], stored_eval["version"]) - images = {(item["folder"], item["key"]): (item["id"], item["version"]) - for item in [*record.get("images", []), *record.get("pulled", [])]} - dockerfiles = {(item["folder"], item["key"]): item.get("dockerfile") for item in record.get("images", [])} - return Imported(entities, images, record.get("files", {}), dockerfiles), None + entities[stored_eval["file"]] = Source(stored_eval["id"], stored_eval["version"]) + images = {(item["folder"], item["key"]): ImportedImage(Source(item["id"], item["version"]), item.get("dockerfile")) + for item in record.get("images", [])} + images |= {(item["folder"], item["key"]): ImportedImage(Source(item["id"], item["version"])) + for item in record.get("pulled", [])} + return Imported(entities, images, record.get("files", {})), None def where(root: Path, entry: BundleEntry) -> str: @@ -67,7 +84,7 @@ def where(root: Path, entry: BundleEntry) -> str: return unicodedata.normalize("NFC", entry.path.relative_to(root).as_posix()) -def source_of(imported: Imported, plan: Plan, write: Write) -> tuple[str, int] | None: +def source_of(imported: Imported, plan: Plan, write: Write) -> Source | None: """The store id and version ``write`` was imported from, or None when the import didn't write it, as for what an installed bundle holds.""" entry = write.source.entry @@ -75,7 +92,8 @@ def source_of(imported: Imported, plan: Plan, write: Write) -> tuple[str, int] | return None folder = where(plan.bundle.bundle.root, entry) if isinstance(write.source, BundleImage) and write.source.id != entry.id: - return imported.images.get((folder, image_key(write.source))) + image = imported.images.get((folder, image_key(write.source))) + return image.source if image is not None else None return imported.entities.get(folder) @@ -96,7 +114,8 @@ def as_imported(imported: Imported, plan: Plan, write: Write, now: Mapping[str, toml = f"{folder}/{CONFIG_FILES[entry.kind]}" if entry.kind in CONFIG_FILES else None then, current = _within(imported.files, folder), _within(now, folder) if isinstance(write.source, BuiltImage) and write.source.id != entry.id: - if imported.dockerfiles.get((folder, image_key(write.source))) != write.source.dockerfile: + image = imported.images.get((folder, image_key(write.source))) + if image is None or image.dockerfile != write.source.dockerfile: return False then.pop(toml, None) current.pop(toml, None) @@ -107,13 +126,101 @@ def as_imported(imported: Imported, plan: Plan, write: Write, now: Mapping[str, return then == current -def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[str]], dict[Path, list[str]]]: +class ImportedWrites: + """What ``imported`` records of each write of ``plan``: the stored id and version it was imported from, and whether + the folder holds it as the import wrote it; and what of the record's tasks ``restored`` couldn't give back. Writes + are told apart by store as well as ``@local`` id, since a task and an env of one name share one.""" + + def __init__(self, imported: Imported | None, plan: Plan, restored: Restored): + self.plan = plan + self.given = restored.given + self._writes = {_key(write): write for write in plan.writes} + self._sources = {_key(write): source for write in plan.writes + if imported is not None and (source := source_of(imported, plan, write)) is not None} + self.tasks = {write.source.entry.id for write in plan.writes + if write.kind is BundleKind.TASK and _key(write) in self._sources} + self.held = _held(imported, plan, [self._writes[key] for key in self._sources]) if imported else frozenset() + self.problems = [problem for write in plan.writes if write.kind is BundleKind.TASK + for problem in restored.unrestored.get(write.source.entry.path, ())] + + def source(self, write: Write) -> Source | None: + return self._sources.get(_key(write)) + + def recorded(self, ids: Mapping[str, str]) -> set[str]: + """The registry ids in ``ids`` the record gives rather than the rules: each imported write's, each imported + task's output's, and each derived from either.""" + taken = {local for _, local in self._sources} + taken |= {local for local in ids if any(local.startswith(f"{task}/") for task in self.tasks)} + return {ids[local] for local in ids if local in taken or any(local.startswith(f"{base}__") for base in taken)} + + def copies(self, store: str, locals_: Sequence[str], registry_id: str) -> list[str] | None: + """When ``locals_``, which take one registry id in ``store``, were imported from one stored entity: a problem + naming those the folder changed, since a registration writes each id once, or none; else None.""" + keys = [(store, local) for local in locals_] + if None in (sources := {self._sources.get(key) for key in keys}) or len(sources) > 1: + return None + paths = sorted(self.plan.path(self._writes[key].source.entry) for key in keys) + if not (edited := sorted(self.plan.path(self._writes[key].source.entry) for key in keys + if key not in self.held)): + return [] + return [f"{', '.join(paths)} hold copies of {registry_id!r} as imported, and {', '.join(edited)} changed it; " + "a registration writes each id once, so undo the change"] + + def versions(self, ids: Mapping[str, str]) -> tuple[dict[tuple[str, str], int], set[tuple[str, str]]]: + """By (store, registry id), the version each imported write was imported at, and which of them the folder holds + as the import wrote it.""" + versions, held = {}, set() + for (store, local), source in self._sources.items(): + key = (store, ids.get(local, local)) + versions[key] = source.version + if (store, local) in self.held: + held.add(key) + return versions, held + + +def _key(write: Write) -> tuple[str, str]: + return (write.kind.store, write.id) + + +def _held(imported: Imported, plan: Plan, writes: list[Write]) -> frozenset[str]: + """The (store, ``@local`` id) of each of ``writes`` the folder holds as ``imported`` records the import wrote it.""" + now = {} + for entry in {write.source.entry for write in writes}: + now |= _files_of(plan.bundle.bundle, entry) + return frozenset(_key(write) for write in writes if as_imported(imported, plan, write, now)) + + +def _files_of(bundle: Bundle, entry: BundleEntry) -> dict[str, str]: + """The files a write of ``entry`` reads, as its writer walks them, by their paths in the bundle, as their + ``file_digest``: a task's or eval's file, or every file of its folder and its toml.""" + folder = where(bundle.root, entry) + if entry.kind in (BundleKind.TASK, BundleKind.EVAL): + return {folder: file_digest(entry.path)} + files = entry_files(bundle, entry, empty_ok=True) + if entry.kind in CONFIG_FILES and (toml := entry.path / CONFIG_FILES[entry.kind]).is_file(): + files[toml.name] = toml + return {f"{folder}/{key}": file_digest(path) for key, path in files.items()} + + +@dataclass(frozen=True) +class Restored: + """A bundle given back what its import left out (``restore``): the paths in each task file's steps that took a + stored value, and a problem for each field still holding what the import masked.""" + + bundle: Bundle + given: Mapping[Path, set[str]] + unrestored: Mapping[Path, list[str]] + + +def restore(bundle: Bundle, imported: Imported | None) -> Restored: """``bundle`` with each imported task's steps given back, from the stored version each was imported from, matched by step id and type, what the import left out where the folder doesn't set it: each user-scoped field, but a reference's pin where the folder names something else; each list item's, from the stored item it was written from; and each env var and credential the import's scrub took, where the folder holds what the scrub left. And, for each task's file, the paths in its steps that took one, and a problem for each field still holding what the scrub left, - which the folder changed too much to give back.""" + which the folder changed too much to give back. A bundle with no record is as it is.""" + if imported is None: + return Restored(bundle, {}, {}) registry = get_task_step_registry() entries, taken, problems = [], {}, {} for entry in bundle.entries: @@ -139,7 +246,7 @@ def grafted(bundle: Bundle, imported: Imported) -> tuple[Bundle, dict[Path, set[ f"{where(bundle.root, entry)}: step {step.get('id')!r}: {path} still holds what the import masked, and " "the folder changed what it was masked from; set its real value" for path in left) entries.append(replace(entry, config=steps)) - return replace(bundle, entries=tuple(entries)), taken, problems + return Restored(replace(bundle, entries=tuple(entries)), taken, problems) def _given_back(step: dict, origin: dict, cls: type) -> tuple[list[str], list[str]]: @@ -165,7 +272,7 @@ def _given_back(step: dict, origin: dict, cls: type) -> tuple[list[str], list[st scrub(scrubbed) given += _unscrubbed(step, origin, scrubbed) masked = {text for _, text in _masks(origin, scrubbed)} - return given, [path for path, text in _texts(step) if text in masked] + return given, [path for path, text in strings(step) if text in masked] def _repinned(step: dict, origin: dict, cls: type) -> set[str]: @@ -181,13 +288,13 @@ def _given_back_items(step: dict, origin: dict, at: str, fields: list[str]) -> l """Give each item of the list ``at`` the ``fields`` the import left out of it, from the stored item it was written from (``_source_item``): all of them, or none when the item sets one, since together they can be one choice, such as a source under either of its names.""" - items, stored = _at(step, at), _at(origin, at) + items, stored = value_at(step, at.split(".")), value_at(origin, at.split(".")) if not isinstance(items, list) or not isinstance(stored, list): return [] left_out = {field.split(".")[0] for field in fields} given = [] for index, item in enumerate(items): - if not isinstance(item, dict) or any(_at(item, field) is not None for field in fields): + if not isinstance(item, dict) or any(value_at(item, field.split(".")) is not None for field in fields): continue if (source := _source_item(item, stored, left_out)) is None: continue @@ -210,13 +317,6 @@ def _bare(item: dict, left_out: set[str]) -> dict: return {key: value for key, value in item.items() if key not in left_out and value is not None} -def _at(step: dict, path: str) -> object: - value = step - for key in path.split("."): - value = value.get(key) if isinstance(value, dict) else None - return value - - def _unscrubbed(value: object, stored: object, scrubbed: object, path: str = "") -> list[str]: """Put back in ``value``, in place, what scrubbing ``stored`` took where ``value`` holds what it left (``scrubbed``): each env var it dropped that ``value`` doesn't set, and each credential it masked; each one's @@ -261,7 +361,7 @@ def _counterpart(item: object, index: int, stored: list, scrubbed: list, unchang same = [other for other, one in enumerate(scrubbed) if one == item] if not same: same = [other for other, one in enumerate(scrubbed) if (masks := _masks(stored[other], one)) - and all(_at_keys(item, keys) == text for keys, text in masks)] + and all(value_at(item, keys) == text for keys, text in masks)] return same[0] if len(same) == 1 else None @@ -277,28 +377,6 @@ def _masks(stored: object, scrubbed: object, keys: tuple = ()) -> list[tuple[tup return [] -def _at_keys(value: object, keys: tuple) -> object: - for key in keys: - if isinstance(value, dict) and isinstance(key, str): - value = value.get(key) - elif isinstance(value, list) and isinstance(key, int) and key < len(value): - value = value[key] - else: - return None - return value - - -def _texts(value: object, path: str = "") -> list[tuple[str, str]]: - """Each string in ``value``, with its path.""" - if isinstance(value, str): - return [(path, value)] - if isinstance(value, dict): - return [text for key, item in value.items() for text in _texts(item, f"{path}.{key}" if path else str(key))] - if isinstance(value, list): - return [text for index, item in enumerate(value) for text in _texts(item, f"{path}[{index}]")] - return [] - - def _graft(step: dict, keys: list[str], value: object) -> bool: """Set ``keys`` in ``step`` to ``value`` when it's unset there; whether it was.""" holder = step diff --git a/src/agent_env/bundle/_values.py b/src/agent_env/bundle/_values.py new file mode 100644 index 000000000..e837e3fe6 --- /dev/null +++ b/src/agent_env/bundle/_values.py @@ -0,0 +1,30 @@ +"""Walks over JSON-like values: each string in one with its path, and the value under a path of keys.""" + +from __future__ import annotations + +from collections.abc import Iterable, Iterator +from typing import Any + + +def strings(value: Any, path: str = "") -> Iterator[tuple[str, str]]: + """Each string in ``value``, with its path: ``a.b`` under a dict, ``a[0]`` in a list.""" + if isinstance(value, str): + yield path, value + elif isinstance(value, dict): + for key, item in value.items(): + yield from strings(item, f"{path}.{key}" if path else str(key)) + elif isinstance(value, list): + for index, item in enumerate(value): + yield from strings(item, f"{path}[{index}]") + + +def value_at(value: Any, keys: Iterable[str | int]) -> Any: + """What ``value`` holds under ``keys``: a dict's item by its key, a list's by its index; None where it holds none.""" + for key in keys: + if isinstance(value, dict) and isinstance(key, str): + value = value.get(key) + elif isinstance(value, list) and isinstance(key, int) and 0 <= key < len(value): + value = value[key] + else: + return None + return value diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 1e8b4df1e..6c1e97c5f 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -37,14 +37,13 @@ from agent_env.store.routing import configured_store, namespace_routing from agent_env.task_step.registry import get_task_step_registry -from ._import_report import file_digest -from ._imported import Imported, as_imported, grafted, read_imported, source_of, where +from ._imported import ImportedWrites, read_imported, restore from ._secrets import credentials_in -from .authoring import entry_files +from ._values import strings from .installed import installed_bundles from .ledger import Check, Ledger from .materialize import Materialization, Materialized, materialize -from .parse import CONFIG_FILES, Bundle, BundleEntry, BundleError, BundleKind, parse_bundle +from .parse import Bundle, BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle, plan_entities from .run import _label from .resolve import BundleImage, Reference, ResolvedBundle, ResolvedEntry, resolve_bundle @@ -91,23 +90,21 @@ def register_bundle( imported, why = read_imported(bundle.root) if why is not None: say(f" {why}") - bundle, grafts, unrestored = grafted(bundle, imported) if imported is not None else (bundle, {}, {}) - resolved = resolve_bundle(bundle) - plan = _plan(resolved, tasks, evals) - if problems := [problem for write in plan.writes if write.kind is BundleKind.TASK - for problem in unrestored.get(write.source.entry.path, ())]: - raise BundleError(problems) - held = _held_as_imported(plan, imported) - ids, problems = registry_ids(plan, _prefix(bundle, prefix), imported, held) + restored = restore(bundle, imported) + plan = _plan(resolve_bundle(restored.bundle), tasks, evals) + imports = ImportedWrites(imported, plan, restored) + if imports.problems: + raise BundleError(imports.problems) + ids, problems = registry_ids(plan, _prefix(bundle, prefix), imports) if problems: raise BundleError(problems) registered = translated(plan, ids) target = configured_store(get_config().get_document_store()) shared = shared_store(target) identity, commit, dirty = bundle_identity(bundle.root) - sources, kept = _sources(plan, ids, imported, held) - ledger = RegistrationLedger(target, registered, identity, commit, dirty, sources, kept) - problems = list(_unregistrable(registered, shared, grafts)) + versions, held = imports.versions(ids) + ledger = RegistrationLedger(target, registered, identity, commit, dirty, versions, held) + problems = list(_unregistrable(registered, shared, imports.given)) if problems: raise BundleError(problems) materialization = materialize( @@ -153,27 +150,25 @@ def _prefix(bundle: Bundle, prefix: str | None) -> str: return f"{name}-" -def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None, - held: frozenset[str] = frozenset()) -> tuple[dict[str, str], list[str]]: +def registry_ids(plan: Plan, prefix: str, + imported: ImportedWrites | None = None) -> tuple[dict[str, str], list[str]]: """The registry id each ``@local`` id the plan writes takes, and each problem giving them. - What ``imported`` records the folder was imported from takes the id it had there, and an imported task's outputs - the names its file gives them, as the stored task did. Any other entry of the bundle's own is ````, its name lowercased with each run of what an id can't hold made - ``-``. An installed bundle's agent pulled in by a reference takes the first alias its bundle gives it, which the - reference named it by; any other entity of an installed bundle has no registry id, and an env's alias can't be - one, since no store env may start on an installed env alias's id. An id derived from another (``__env_image``) keeps its suffix after the - other's registry id, and a task's output is ``-``.""" + What ``imported`` records the folder was imported from takes the id it had there, and an imported task's outputs the + names its file gives them, as the stored task did. Any other entry of the bundle's own is ````, its + name lowercased with each run of what an id can't hold made ``-``. An installed bundle's agent pulled in by a + reference takes the first alias its bundle gives it, which the reference named it by; any other entity of an + installed bundle has no registry id, and an env's alias can't be one, since no store env may start on an installed + env alias's id. An id derived from another (``__env_image``) keeps its suffix after the other's registry id, and + a task's output is ``-``.""" ids: dict[str, str] = {} problems: list[str] = [] - imported_tasks = set() for write in plan.writes: entry = write.source.entry - if imported is not None and (source := source_of(imported, plan, write)) is not None: - if ids.setdefault(write.id, source[0]) != source[0]: + if imported is not None and (source := imported.source(write)) is not None: + if ids.setdefault(write.id, source.id) != source.id: problems.append(f"{plan.path(entry)}: shares its name with what's registered as {ids[write.id]!r}, " - f"so it can't keep the id it was imported as, {source[0]!r}; rename one") - continue - imported_tasks.update([entry.id] if write.kind is BundleKind.TASK else []) + f"so it can't keep the id it was imported as, {source.id!r}; rename one") continue if entry.id in ids or (isinstance(write.source, BundleImage) and write.id != entry.id): continue @@ -195,7 +190,7 @@ def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None, if isinstance(source, ResolvedEntry) and source.entry.id in ids: for output in source.outputs: written = output.id.removeprefix(f"{source.entry.id}/") - ids[output.id] = (written if source.entry.id in imported_tasks + ids[output.id] = (written if imported is not None and source.entry.id in imported.tasks else f"{ids[source.entry.id]}-{_slug(written.rsplit('/', 1)[-1])}") stores: dict[str, set[str]] = {} for write in plan.writes: @@ -204,76 +199,22 @@ def registry_ids(plan: Plan, prefix: str, imported: Imported | None = None, for output in write.source.outputs: stores.setdefault(output.id, set()).add(BundleKind.ARTIFACT.store) by_target: dict[tuple[str, str], list[str]] = {} - kept = {registry_id for local, registry_id in ids.items() - if imported is not None and local not in _given(ids, imported_tasks, plan, imported)} + recorded = imported.recorded(ids) if imported is not None else set() for local, registry_id in ids.items(): for store in stores.get(local, ()): by_target.setdefault((store, registry_id), []).append(local) - if registry_id not in kept and not _REGISTRY_ID.fullmatch(registry_id): + if registry_id not in recorded and not _REGISTRY_ID.fullmatch(registry_id): problems.append(f"{registry_id!r}: isn't an id a registry takes; rename what it's made from") - copies = {write.id: (write, source_of(imported, plan, write)) for write in plan.writes} if imported else {} - for (_, registry_id), locals_ in by_target.items(): + for (store, registry_id), locals_ in by_target.items(): if len(locals_) < 2: continue - found = [copies.get(local, (None, None)) for local in locals_] - if None in {source for _, source in found} or len({source for _, source in found}) > 1: + if imported is not None and (copies := imported.copies(store, locals_, registry_id)) is not None: + problems += copies + else: problems.append(f"{', '.join(sorted(locals_))} would all be registered as {registry_id!r}; rename one") - elif edited := sorted(plan.path(write.source.entry) for write, _ in found if write.id not in held): - problems.append(f"{', '.join(sorted(plan.path(write.source.entry) for write, _ in found))} hold copies of " - f"{registry_id!r} as imported, and {', '.join(edited)} changed it; a registration writes " - "each id once, so undo the change") return ids, problems -def _given(ids: Mapping[str, str], imported_tasks: set[str], plan: Plan, imported: Imported) -> set[str]: - """The ``@local`` ids in ``ids`` whose registry ids the rules made, not ``imported``'s record: what isn't an - imported write, an imported task's output, or derived from either.""" - taken = {write.id for write in plan.writes if source_of(imported, plan, write) is not None} - taken |= {local for local in ids if any(local.startswith(f"{task}/") for task in imported_tasks)} - return {local for local in ids - if local not in taken and not any(local.startswith(f"{base}__") for base in taken)} - - -def _held_as_imported(plan: Plan, imported: Imported | None) -> frozenset[str]: - """The ``@local`` id of each write of ``plan`` that the folder holds as ``imported`` records the import wrote it.""" - if imported is None: - return frozenset() - writes = [write for write in plan.writes if source_of(imported, plan, write) is not None] - now = {} - for entry in {write.source.entry for write in writes}: - now |= _files_of(plan.bundle.bundle, entry) - return frozenset(write.id for write in writes if as_imported(imported, plan, write, now)) - - -def _files_of(bundle: Bundle, entry: BundleEntry) -> dict[str, str]: - """The files a write of ``entry`` reads, as its writer walks them, by their paths in the bundle, as their - ``file_digest``: a task's or eval's file, or every file of its folder and its toml.""" - folder = where(bundle.root, entry) - if entry.kind in (BundleKind.TASK, BundleKind.EVAL): - return {folder: file_digest(entry.path)} - files = entry_files(bundle, entry, empty_ok=True) - if entry.kind in CONFIG_FILES and (toml := entry.path / CONFIG_FILES[entry.kind]).is_file(): - files[toml.name] = toml - return {f"{folder}/{key}": file_digest(path) for key, path in files.items()} - - -def _sources(plan: Plan, ids: Mapping[str, str], imported: Imported | None, - held: frozenset[str]) -> tuple[dict[tuple[str, str], int], set[tuple[str, str]]]: - """For each write of ``plan``, under its ``@local`` ids, that the folder was imported from, by (store, registry id), - the version it was imported at; and which of them the folder still holds as the import wrote it (``held``).""" - if imported is None: - return {}, set() - sources, kept = {}, set() - for write in plan.writes: - if (source := source_of(imported, plan, write)) is None: - continue - key = (write.kind.store, ids.get(write.id, write.id)) - sources[key] = source[1] - if write.id in held: - kept.add(key) - return sources, kept - - def _alias(bundle: Bundle, entry: BundleEntry) -> str | None: aliases = bundle.aliases.get(AGENT_ALIASES, {}).get(entry.name, ()) if entry.kind is BundleKind.AGENT else () return aliases[0] if aliases else None @@ -457,7 +398,7 @@ def _unregistrable(plan: Plan, shared: bool, grafts: Mapping[Path, set[str]]) -> def stored(path: str) -> bool: return any(path == field or path.startswith((f"{field}.", f"{field}[")) for field in taken) - for path, text in _strings(write.source.config): + for path, text in strings(write.source.config): if is_local_id(text): yield f"{where}: {path or 'its value'} names {text!r}, which only this machine's stores hold" elif shared and not stored(path) and (url := _local_url(text)) is not None: @@ -468,17 +409,6 @@ def stored(path: str) -> bool: yield f"{where}: {path}: {why}; leave it out, for whatever runs the task to give it" -def _strings(value: Any, path: str = "") -> Iterator[tuple[str, str]]: - if isinstance(value, str): - yield path, value - elif isinstance(value, dict): - for key, item in value.items(): - yield from _strings(item, f"{path}.{key}" if path else str(key)) - elif isinstance(value, list): - for index, item in enumerate(value): - yield from _strings(item, f"{path}[{index}]") - - def _local_url(text: str) -> str | None: """The first URL in ``text`` that reaches only this machine: a file, or a loopback host's.""" for match in _URL.finditer(text): diff --git a/tst/unit/bundle/register_imports_test.py b/tst/unit/bundle/register_imports_test.py index e4e0a3c18..129ad217d 100644 --- a/tst/unit/bundle/register_imports_test.py +++ b/tst/unit/bundle/register_imports_test.py @@ -15,7 +15,7 @@ from agent_env.bundle import _imported as imported_module from agent_env.bundle import materialize as materialize_module from agent_env.bundle import register as register_module -from agent_env.bundle._imported import Imported, source_of +from agent_env.bundle._imported import Imported, ImportedImage, Source, source_of from agent_env.bundle.importer import import_task from agent_env.bundle.installed import InstalledBundle from agent_env.bundle.parse import parse_bundle @@ -316,7 +316,8 @@ def test_an_installed_bundle_s_agent_takes_no_source_from_the_folder_s_record(lo folder = layout(tmp_path / "triage", {"tasks/t.json": json.dumps([ {"id": "agent", "type": "deploy_agent", "a2a_agent_id": "@local/demo-dist/kit/solver"}])}) plan = plan_bundle(resolve_bundle(parse_bundle(folder)), installed=lambda: (kit,)) - record = Imported({"agents/solver": ("solver", 3)}, {("agents/solver", "image"): ("solver-image", 3)}, {}, {}) + record = Imported({"agents/solver": Source("solver", 3)}, + {("agents/solver", "image"): ImportedImage(Source("solver-image", 3))}, {}) installed = [write for write in plan.writes if write.source.entry in plan.origins]