From 6bdf20a71b0aba3451cc44ce8cde0b1fff720d4a Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 15:19:26 -0700 Subject: [PATCH 1/6] feat(bundle): a bundle.toml names the plugin packages a bundle requires A bundle's optional bundle.toml lists the pip requirements it needs (requires = ["agentenv-browser>=1.2"]). Resolving a bundle refuses one this environment doesn't meet, before anything runs, naming the one agent-env plugin add command that installs them; in a terminal, run and bundle register offer to run it and then run the command again. A run or registration of a writable folder run by its path that has no bundle.toml creates one from the plugin distributions whose types the bundle uses, at least the version installed, never over a file that appeared meanwhile. A dry run says what it would write. An existing bundle.toml is never edited: the run says what to add, so requires = [] opts out. A type only configuration registers gets a hint, never a requirement. bundle init --from-task writes the same file. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 9 + pyproject.toml | 2 +- src/agent_env/bundle/_import_report.py | 1 + src/agent_env/bundle/importer.py | 48 +-- src/agent_env/bundle/parse.py | 49 ++- src/agent_env/bundle/register.py | 10 +- src/agent_env/bundle/requires.py | 244 +++++++++++ src/agent_env/bundle/resolve.py | 8 +- src/agent_env/bundle/run.py | 27 +- src/agent_env/cli/_install_prompt.py | 47 +++ src/agent_env/cli/bundle.py | 18 +- src/agent_env/cli/run.py | 8 + tst/unit/bundle/importer_test.py | 11 +- tst/unit/bundle/parse_test.py | 44 +- tst/unit/bundle/requires_test.py | 443 ++++++++++++++++++++ tst/unit/bundle/run_test.py | 24 +- tst/unit/config/test_explain_and_sources.py | 3 +- 17 files changed, 927 insertions(+), 69 deletions(-) create mode 100644 src/agent_env/bundle/requires.py create mode 100644 src/agent_env/cli/_install_prompt.py create mode 100644 tst/unit/bundle/requires_test.py diff --git a/AGENTS.md b/AGENTS.md index 54a9f6081..340d3ef13 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -177,6 +177,15 @@ Out-of-tree code plugs in either through `config.toml`, as a `module:Class` poin `A2AAgent.validate()` deploys `mail` (`A2AAgent.VALIDATION_ENV_ID`) with namespace routing on, first writing the example (its writes and infra, no run) when the store has no env of that id: on a remote sandbox its image is context-only and built in the VM, so a service like the hub's worker needs no Docker. +- A bundle names the plugin packages it needs in an optional `bundle.toml` at its root (`requires = ["", ...]`, read by `bundle/parse.py`, checked by `bundle/requires.py`). Resolving a bundle refuses one + whose requirements this environment doesn't meet (`UnmetRequirements`), naming the `agent-env plugin add` command + that installs them; in a terminal `agent-env run` and `bundle register` offer to run it, then run the command again + in a new process. A run or registration of a writable folder run by its path that has no `bundle.toml` creates one, + requiring at least the installed version of each plugin distribution whose type the bundle uses, never over a file + that appeared meanwhile. An existing one is never edited: the run says what to add, so `requires = []` keeps a + bundle requiring nothing. A type only configuration registers gets a hint, never a requirement. `bundle init + --from-task` writes the same file. - Environment providers have no table: an installed package registers one as an `agent_env.env_providers` entry point named its `type`, which records carry as `env_provider_type`. - `[plugins.]` is reserved for an installed plugin's own settings, read with diff --git a/pyproject.toml b/pyproject.toml index d9eb26d1e..8627fc077 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ dependencies = [ "httpx>=0.27.0", "litellm>=1.96.0,<1.97", "mcp>=1.25,<2", # same floor as env/gateway/requirements.txt and agentenv-protocol; 2.0.0 removed mcp.server.fastmcp, imported at module scope by env/gateway - "packaging>=22", # evaluates a plugin's requirement markers with no extra + "packaging>=22", # parses a bundle.toml's requires; evaluates a plugin's requirement markers with no extra "a2a-sdk==0.3.26", "pydantic>=2.12", # model_validate(extra=...) reads agent responses tolerantly "pymongo>=4.7", diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index 108c2a79b..c8b941286 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -58,6 +58,7 @@ class Need: what: str # step type, env type or artifact type name: str by: str # the installed distribution that registers it, or "configuration" for a config file's impls + version: str | None = None # the distribution's, installed here @dataclass(frozen=True) diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index c1c6d1f55..0cf62d32f 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -4,7 +4,6 @@ from __future__ import annotations import contextlib -import importlib import json import os import shutil @@ -19,7 +18,7 @@ from agent_env.artifact.artifact import Artifact from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.artifact.artifacts.skill import SkillArtifact -from agent_env.artifact.registry import ARTIFACT_REGISTRY, canonical_type, get_artifact_registry +from agent_env.artifact.registry import canonical_type, get_artifact_registry from agent_env.bundle import _toml from agent_env.bundle._fs import fold, show from agent_env.bundle._images import ImageJob, context_groups, write_images @@ -42,6 +41,7 @@ from agent_env.bundle._secrets import scrub from agent_env.bundle.installed import InstalledBundle, installed_bundles from agent_env.bundle.parse import ( + BUNDLE_TOML, CONFIG_FILES, LEAVINGS, BundleError, @@ -51,21 +51,20 @@ segment_problem, ) from agent_env.bundle.plan import STORE_GETTERS, check_bundle +from agent_env.bundle.requires import Provider, Providers from agent_env.bundle.resolve import resolve_bundle from agent_env.config import get_config from agent_env.entity.export import ExportContext, Unwritable, checked_path from agent_env.entity.refs import EntityKind, EntityRef, RefRole, RefSite, default_agent_refs, ref_sites from agent_env.entity.scopes import FieldScope, prune -from agent_env.env import registry as env_registry from agent_env.env.env import Env from agent_env.env.registry import get_env_registry from agent_env.eval import Eval -from agent_env.plugins import ARTIFACTS, ENVS, TASK_STEPS, inventory, load_failures +from agent_env.plugins import ARTIFACTS, ENVS, TASK_STEPS from agent_env.store.base import NotFoundError from agent_env.store.ids import LOCAL_PREFIX from agent_env.store.image_store.oci_registry_credentials import names_registry from agent_env.task.task import Task -from agent_env.task_step import registry as task_step_registry from agent_env.task_step.registry import get_task_step_registry from agent_env.utils.paths import os_reason @@ -180,7 +179,7 @@ def __init__(self, dest: Path, include_scopes: Collection[FieldScope] = (), self.tomls: dict[str, tuple[str, dict]] = {} # each entity folder's toml: its file's name, and its keys self.downloads: dict[str, str] = {} # each file to write: its path in the bundle, and its object's url self.jobs: list[ImageJob] = [] - self.origins: dict[tuple[str, str], str] | None = None + self.providers = Providers() self.installed: tuple[InstalledBundle, ...] | None = None _check_destination(self.dest) @@ -288,6 +287,9 @@ def write(self, description: str, source: dict) -> BundleImport: if self.problems: raise BundleError(self.problems) shutil.rmtree(scratch) + if requires := sorted({requirement for need in self.needs if need.by != "configuration" + and (requirement := Provider(need.by, need.version).requirement())}): + self.files[BUNDLE_TOML] = _toml.dumps({"requires": requires}) files = sorted([*self.files, PROVENANCE, "README.md"]) imported = BundleImport( root=self.dest, files=tuple(files), tasks=tuple(self.tasks), eval=self.eval, @@ -477,19 +479,10 @@ def _need_type(self, site: RefSite, entity: Any, where: str) -> None: get_artifact_registry().get(name)) def _need(self, where: str, what: str, group: str, name: str, cls: type | None) -> None: - """``name``, of the plugin ``group``, is a need when core doesn't ship it, named with what registers ``cls``, - the class it resolves to here: the distribution whose entry point names that class, or configuration. A plugin - the registry build skipped registers nothing, so it isn't loaded again to compare.""" - if name in _BUILTINS[group](): - return - if self.origins is None: - self.origins = {(contribution.group, contribution.name): (distribution.name, contribution.value) - for distribution in inventory(load=False).distributions - for contribution in distribution.contributions} - distribution, value = self.origins.get((group, name), (None, None)) - registered = value is not None and cls is not None and name not in load_failures().get(group, {}) - by = distribution if registered and _names(value, cls) else "configuration" - self.needs.append(Need(where, what, name, by)) + """``name``, of the plugin ``group``, is a need when core doesn't ship it, with what provides ``cls``, the class + it resolves to here (``Providers.of``).""" + if (provider := self.providers.of(group, name, cls)) is not None: + self.needs.append(Need(where, what, name, provider.distribution or "configuration", provider.version)) def _default_agents(self, step: dict, refs: tuple[EntityRef, ...], where: str) -> None: """A step that leaves an agent field unset falls back to the configured default, which no field pins.""" @@ -511,11 +504,6 @@ def _name(self, kind: str, entity_id: str) -> str: return name -# What core ships, per plugin group: anything else a task uses is a need. -_BUILTINS = {TASK_STEPS: task_step_registry._builtin_registry, ENVS: env_registry._builtin_registry, - ARTIFACTS: lambda: ARTIFACT_REGISTRY} - - def _names_of(kind: BundleKind) -> str: """The folder whose names ``kind``'s share: artifacts' and skills' are one.""" return BundleKind.ARTIFACT.value if kind is BundleKind.SKILL else kind.value @@ -549,18 +537,6 @@ def _fetch(url: str, dest: Path) -> None: get_config().get_object_store_at(url).download_to_file(url, str(dest)) -def _names(value: str, cls: type) -> bool: - """Whether the entry point ``value``, ``module:attribute``, names ``cls``.""" - module, _, attribute = value.partition(":") - try: - named = importlib.import_module(module) - for part in attribute.split("."): - named = getattr(named, part) - except (ImportError, AttributeError): - return False - return named is cls - - def _output_names(sites: list[RefSite]) -> dict[tuple[EntityKind, str], str]: """The name each output takes: as written, or an ``@local`` id's last segment, numbered when it is taken.""" written = {_key(site) for site in sites if site.ref.role is RefRole.OUTPUT} diff --git a/src/agent_env/bundle/parse.py b/src/agent_env/bundle/parse.py index c6b39184a..faf14feec 100644 --- a/src/agent_env/bundle/parse.py +++ b/src/agent_env/bundle/parse.py @@ -21,6 +21,8 @@ from pathlib import Path from typing import Any, NoReturn +from packaging.requirements import InvalidRequirement, Requirement + from agent_env.bundle._fs import Unreadable, fold, home, on_disk, read_regular, show, with_article from agent_env.entity.refs import EntityKind from agent_env.plugins._bundles import AGENT_ALIASES, ALIASES_FILE, ARTIFACT_ALIASES, ENV_ALIASES, read_aliases @@ -87,6 +89,8 @@ class Bundle: # The aliases its aliases.toml gives its agents and envs, by table and then folder name, which a reference names # one by once the bundle is installed. aliases: Mapping[str, Mapping[str, tuple[str, ...]]] = field(default_factory=dict) + # The plugin packages its bundle.toml requires, each a pip requirement; None when it has no bundle.toml. + requires: tuple[str, ...] | None = None class BundleError(ValueError): @@ -127,6 +131,7 @@ def __init__(self, root: Path, id_root: str | None, name: str | None): self.problems: list[str] = [] self.ignored: list[Path] = [] self.aliases_file: Path | None = None + self.bundle_toml: Path | None = None self.entry_point = id_root is not None self.root_ok = True if id_root is None: @@ -145,6 +150,7 @@ def parse(self) -> Bundle: entries = [entry for kind, path in kinds.items() for entry in self._scan_kind(kind, path)] self._check_unique(entries) aliases = self._aliases(entries) + requires = self._requires() if self.problems: raise BundleError(self.problems) entries.sort(key=lambda entry: (_ORDER[entry.kind], entry.name)) @@ -156,6 +162,7 @@ def parse(self) -> Bundle: entries=tuple(entries), ignored=tuple(sorted(self.ignored)), aliases=aliases, + requires=requires, ) def _aliases(self, entries: list[BundleEntry]) -> dict[str, dict[str, tuple[str, ...]]]: @@ -187,6 +194,37 @@ def _aliases(self, entries: list[BundleEntry]) -> dict[str, dict[str, tuple[str, f"{table}.{name}: {alias!r} is the {noun} {other.name!r}'s {what}") return declared + def _requires(self) -> tuple[str, ...] | None: + """The plugin packages the bundle's bundle.toml requires, each a pip requirement naming a package by name and + version, not by URL; None when it has no bundle.toml.""" + if self.bundle_toml is None: + return None + try: + data = tomllib.loads(self.bundle_toml.read_text(encoding="utf-8")) + except OSError as e: + self._problem(self.bundle_toml, f"can't be read: {os_reason(e)}") + return () + except (UnicodeDecodeError, tomllib.TOMLDecodeError) as e: + self._problem(self.bundle_toml, f"isn't valid TOML: {e}") + return () + for key in sorted(data.keys() - {"requires"}): + self._problem(self.bundle_toml, f"{key!r} isn't a key it takes; it takes requires") + requires = data.get("requires", []) + if not isinstance(requires, list) or not all(isinstance(line, str) for line in requires): + self._problem(self.bundle_toml, 'requires must be a list of pip requirements, such as ' + '["agentenv-browser>=1.2"]') + return () + for line in requires: + try: + requirement = Requirement(line) + except InvalidRequirement as e: + self._problem(self.bundle_toml, f"requires: {line!r} isn't a pip requirement: {e}") + continue + if requirement.url: + self._problem(self.bundle_toml, f"requires: {line!r} names a URL; name the package and the versions " + "it needs, which install from wherever whoever runs the bundle installs from") + return tuple(requires) + def _derive_id_root(self) -> str: id_root, problems = _id_root(self.root) for path, message in problems: @@ -212,9 +250,14 @@ def _scan_root(self) -> tuple[dict[BundleKind, Path], str | None]: for entry in listing: if entry.name in misspelled: continue - if fold(entry.name) == fold(BUNDLE_TOML): - self._problem(entry.path, "reserved: a bundle takes its name from the folder and its " - "description from README.md") + if entry.name == BUNDLE_TOML: + node = self._node(Path(entry.path), entry) + if node == _FILE: + self.bundle_toml = Path(entry.path) + elif node == _DIR: + self._problem(entry.path, "must be a file, not a folder") + elif fold(entry.name) == fold(BUNDLE_TOML): + self._problem(entry.path, f"spell it {BUNDLE_TOML}, the file a bundle declares the plugins it needs in") elif entry.name == ALIASES_FILE: node = self._node(Path(entry.path), entry) if node == _FILE: diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index cb2477bde..62012641a 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -43,6 +43,7 @@ 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 .requires import declare, writable from .run import _label from .resolve import BundleImage, Reference, ResolvedBundle, ResolvedEntry, resolve_bundle @@ -64,6 +65,7 @@ class Registration: 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 + requires: tuple[str, ...] # what to say of the plugin packages it uses that its bundle.toml doesn't require def register_bundle( @@ -73,12 +75,14 @@ def register_bundle( evals: Sequence[str] = (), prefix: str | None = None, dry_run: bool = False, + write_requires: bool | None = None, 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. 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. + is given a line for each write, as it's checked or written, and before each image is built. A folder with no + bundle.toml that uses plugin packages is given one when ``write_requires``, by default when it isn't a dry run. 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,13 +101,15 @@ def register_bundle( problems = list(_unregistrable(registered, shared)) if problems: raise BundleError(problems) + writes = not dry_run if write_requires is None else write_requires + requires = tuple(declare(resolved, own=writable(bundle.root), dry_run=not writes)) say = on_progress or (lambda line: None) materialization = materialize( 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)}")) - return Registration(registered, materialization, ids, tuple(_floating(registered)), identity, shared) + return Registration(registered, materialization, ids, tuple(_floating(registered)), identity, shared, requires) def shared_store(store: DocumentStore) -> bool: diff --git a/src/agent_env/bundle/requires.py b/src/agent_env/bundle/requires.py new file mode 100644 index 000000000..50b90d5ea --- /dev/null +++ b/src/agent_env/bundle/requires.py @@ -0,0 +1,244 @@ +"""What a bundle needs beyond core: the plugin packages its optional ``bundle.toml`` requires, checked against what's +installed before the bundle resolves, and who provides each type it uses that core doesn't ship, which a bundle with no +``bundle.toml`` is given one from.""" + +from __future__ import annotations + +import importlib +import json +import os +import shlex +import uuid +from collections.abc import Iterable, Sequence +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING + +from packaging.requirements import Requirement +from packaging.utils import canonicalize_name + +from agent_env.artifact.registry import ARTIFACT_REGISTRY, canonical_type, get_artifact_registry +from agent_env.env import registry as env_registry +from agent_env.env.registry import get_env_registry +from agent_env.plugins import ARTIFACTS, ENVS, TASK_STEPS, inventory, load_failures +from agent_env.plugins._requirements import installed +from agent_env.task_step import registry as task_step_registry +from agent_env.task_step.registry import get_task_step_registry + +from . import _toml +from ._fs import relative +from .parse import BUNDLE_TOML, BundleError, BundleKind + +if TYPE_CHECKING: + from .resolve import ResolvedBundle + +# What core ships, per plugin group: anything else a bundle uses is provided by a plugin, or by configuration. +_BUILTINS = {TASK_STEPS: task_step_registry._builtin_registry, ENVS: env_registry._builtin_registry, + ARTIFACTS: lambda: ARTIFACT_REGISTRY} + + +class UnmetRequirements(BundleError): + """A bundle whose ``bundle.toml`` requires packages that aren't installed, or not at a version it allows, with + every other problem resolving it found; ``requirements`` are the unmet ones, as it writes them.""" + + def __init__(self, problems: Iterable[str], requirements: Sequence[str]): + super().__init__(problems) + self.requirements = tuple(requirements) + + +@dataclass(frozen=True) +class Provider: + """What registers a type core doesn't ship: an installed distribution, at its version, or, with neither, the + configuration's impls.""" + + distribution: str | None + version: str | None + + def requirement(self) -> str | None: + """The pip requirement for it, at least the version installed here; None for configuration.""" + return f"{self.distribution}>={self.version}" if self.distribution else None + + +@dataclass(frozen=True) +class Use: + """A type a bundle uses that core doesn't ship: where, what kind of type, its name, and what provides it.""" + + where: str + what: str # step type, env type or artifact type + name: str + provider: Provider + + +class Providers: + """What provides each type core doesn't ship, read once from the installed plugins' metadata.""" + + def __init__(self) -> None: + self._origins: dict[tuple[str, str], tuple[str, str, str]] | None = None + + def of(self, group: str, name: str, cls: type | None) -> Provider | None: + """What provides ``name`` of the plugin ``group``, which resolves here to ``cls``: the distribution whose entry + point names that class, or configuration; None when core ships it. A plugin the registry build skipped + registers nothing, so it isn't loaded again to compare.""" + if name in _BUILTINS[group](): + return None + if self._origins is None: + self._origins = {(contribution.group, contribution.name): (distribution.name, distribution.version, + contribution.value) + for distribution in inventory(load=False).distributions + for contribution in distribution.contributions} + distribution, version, value = self._origins.get((group, name), (None, None, None)) + registered = value is not None and cls is not None and name not in load_failures().get(group, {}) + return Provider(distribution, version) if registered and _names(value, cls) else Provider(None, None) + + +def unmet(requires: Iterable[str]) -> list[tuple[str, str]]: + """Each of ``requires`` that isn't met here, with why: its package isn't installed, or is at a version it excludes. + One whose marker is false here doesn't apply.""" + found = [] + for line in requires: + requirement = Requirement(line) + if requirement.marker is not None and not requirement.marker.evaluate({"extra": ""}): + continue + have = installed(canonicalize_name(requirement.name)) + if have is None: + found.append((line, "which isn't installed")) + elif not requirement.specifier.contains(have, prereleases=True): + found.append((line, f"and {requirement.name} {have} is installed")) + return found + + +def unmet_problems(requires: Iterable[str]) -> tuple[list[str], list[str]]: + """The problems ``bundle.toml``'s unmet ``requires`` raise, naming the one command that installs them all, and the + unmet requirements.""" + found = unmet(requires) + if not found: + return [], [] + lines = [line for line, _ in found] + command = install_command(lines) + if len(found) == 1: + line, why = found[0] + return [f"{BUNDLE_TOML}: requires {line!r}, {why}; install it with: {command}"], lines + return ([f"{BUNDLE_TOML}: requires {line!r}, {why}" for line, why in found] + + [f"{BUNDLE_TOML}: install what it requires with: {command}"]), lines + + +def install_command(requirements: Sequence[str]) -> str: + """The command that installs ``requirements`` into this environment, through the installer that owns it.""" + return shlex.join(["agent-env", "plugin", "add", *requirements]) + + +def uses(resolved: ResolvedBundle, providers: Providers | None = None) -> list[Use]: + """Each type the bundle's own tasks, envs and artifacts use that core doesn't ship.""" + providers = providers or Providers() + steps, envs, artifacts = get_task_step_registry(), get_env_registry(), get_artifact_registry() + found = [] + for entry in resolved.entries: + where = relative(resolved.bundle.root, entry.entry.path) + kind = entry.entry.kind + if kind is BundleKind.TASK: + for step in entry.config: + step_type = step.get("type") + if isinstance(step_type, str) and (provider := providers.of(TASK_STEPS, step_type, + steps.get(step_type))) is not None: + found.append(Use(f"{where}: step {step.get('id')!r}", "step type", step_type, provider)) + elif kind is BundleKind.ENV and (provider := providers.of(ENVS, entry.entry.type, + envs.get(entry.entry.type))) is not None: + found.append(Use(where, "env type", entry.entry.type, provider)) + elif kind in (BundleKind.ARTIFACT, BundleKind.SKILL): + name = canonical_type(entry.entry.type) + if (provider := providers.of(ARTIFACTS, name, artifacts.get(name))) is not None: + found.append(Use(where, "artifact type", name, provider)) + return found + + +def detected(found: Iterable[Use]) -> list[str]: + """The requirement for each distinct plugin distribution ``found`` names, at least the version installed here.""" + return sorted({requirement for use in found if (requirement := use.provider.requirement()) is not None}) + + +def declare(resolved: ResolvedBundle, *, own: bool, dry_run: bool) -> list[str]: + """What to say of the plugin packages the bundle uses that its ``bundle.toml`` doesn't require, and of each type + only this machine's configuration registers. A bundle with no ``bundle.toml`` in a folder of its ``own`` (run by + path, and writable) is given one listing them, created whole or not at all and never over one that appeared + meanwhile; a ``dry_run`` says what it would write instead. Any other bundle is told what to declare: an existing + ``bundle.toml`` is never edited.""" + found = uses(resolved) + declared = {canonicalize_name(Requirement(line).name) for line in resolved.bundle.requires or ()} + undeclared = [use for use in found if use.provider.distribution is not None + and canonicalize_name(use.provider.distribution) not in declared] + configured = [use for use in found if use.provider.distribution is None] + lines = [f"this bundle uses {_listing(configured)} ({_wheres(configured)}), which this machine's configuration " + "registers; whoever runs it needs that configuration too"] if configured else [] + if not undeclared: + return lines + requires = detected(undeclared) + path = resolved.bundle.root / BUNDLE_TOML + if resolved.bundle.requires is None and own: + if dry_run: + return [f"would write {BUNDLE_TOML}: requires = {json.dumps(requires)}", *lines] + if _create(path, _toml.dumps({"requires": requires})): + return [f"wrote {BUNDLE_TOML}: requires = {json.dumps(requires)}; commit it so whoever you share the " + "bundle with knows what to install", *lines] + by_distribution: dict[str, list[Use]] = {} + for use in undeclared: + by_distribution.setdefault(use.provider.distribution, []).append(use) + hints = [f"this bundle uses {distribution} {uses_of[0].provider.version} ({_wheres(uses_of)}), which " + f"{BUNDLE_TOML} doesn't require" for distribution, uses_of in sorted(by_distribution.items())] + everything = sorted({*(resolved.bundle.requires or ()), *requires}) + hints.append(f"declare them in {BUNDLE_TOML}, so whoever it's shared with knows what to install: " + f"requires = {json.dumps(everything)}") + return [*hints, *lines] + + +def writable(root: Path) -> bool: + """Whether a file can be created in the folder ``root``.""" + return os.access(root, os.W_OK | os.X_OK) + + +def _create(path: Path, text: str) -> bool: + """Write ``text`` to ``path`` whole, unless something is there by then; whether it was written. It's staged beside + it under a name of its own, as the umask leaves new files, and linked into place, which fails if ``path`` exists.""" + staged = path.with_name(f".{path.name}.{uuid.uuid4().hex}.tmp") + try: + fd = os.open(staged, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o666) + except OSError: + return False + try: + with os.fdopen(fd, "w", encoding="utf-8") as file: + file.write(text) + os.link(staged, path) + return True + except FileExistsError: + return False + except OSError: # a filesystem without links: create it in place, still never over another's + try: + with open(path, "x", encoding="utf-8") as file: + file.write(text) + return True + except OSError: + return False + finally: + staged.unlink(missing_ok=True) + + +def _listing(found: Sequence[Use]) -> str: + """Each distinct type ``found`` names, once: ``the step type 'a' and the env type 'b'``.""" + named = list(dict.fromkeys(f"the {use.what} {use.name!r}" for use in found)) + return named[0] if len(named) == 1 else f"{', '.join(named[:-1])} and {named[-1]}" + + +def _wheres(found: Sequence[Use]) -> str: + more = len(found) - 1 + return found[0].where + (f", and {more} more" if more else "") + + +def _names(value: str, cls: type) -> bool: + """Whether the entry point ``value``, ``module:attribute``, names ``cls``.""" + module, _, attribute = value.partition(":") + try: + named = importlib.import_module(module) + for part in attribute.split("."): + named = getattr(named, part) + except (ImportError, AttributeError): + return False + return named is cls diff --git a/src/agent_env/bundle/resolve.py b/src/agent_env/bundle/resolve.py index 33a76567b..fd0cce0e5 100644 --- a/src/agent_env/bundle/resolve.py +++ b/src/agent_env/bundle/resolve.py @@ -33,6 +33,7 @@ from ._fs import fold, relative, show from .authoring import AuthoringContext, default_dockerfile from .parse import DOCKERFILE, NAMED_BY, Bundle, BundleEntry, BundleError, BundleKind +from .requires import UnmetRequirements, unmet_problems # Step keys that hold step ids, never entity ids. _STEP_ID_KEYS = ("id", "type", "depends_on") @@ -107,7 +108,8 @@ class ResolvedBundle: def resolve_bundle(bundle: Bundle) -> ResolvedBundle: - """Resolve every entry's references. Raises BundleError listing every problem found.""" + """Resolve every entry's references. Raises BundleError listing every problem found, UnmetRequirements when the + bundle.toml requires a package that isn't installed or not at a version it allows.""" return _Resolver(bundle).resolve() @@ -169,6 +171,8 @@ def __init__(self, bundle: Bundle): self.ignored[kind, fold(name)] = path def resolve(self) -> ResolvedBundle: + problems, unmet = unmet_problems(self.bundle.requires or ()) + self.problems.extend(problems) resolved = [] for entry in self.bundle.entries: if self._image_folder(entry): @@ -178,7 +182,7 @@ def resolve(self) -> ResolvedBundle: except _Skip: pass if self.problems: - raise BundleError(self.problems) + raise UnmetRequirements(self.problems, unmet) if unmet else BundleError(self.problems) return ResolvedBundle(self.bundle, tuple(resolved), tuple(self.images)) # One method per kind of config diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index 1f1786675..a384004ff 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -39,6 +39,7 @@ from .parse import BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle from .preflight import Preflight, preflight_run +from .requires import declare, writable from .resolve import BuiltImage, BundleImage, Reference, ReferencedImage, resolve_bundle, without_played_agents logger = logging.getLogger(__name__) @@ -240,7 +241,7 @@ def run_bundle( build_sandbox_provider(sandbox) say = _progress(on_progress) with namespace_routing(): - prepared = _prepare(root, tasks, evals, sandbox, id_root, agent, False, say) + prepared = _prepare(root, tasks, evals, sandbox, id_root, agent, False, say, own=_own(root, id_root)) plan, entries = prepared.plan, [entry for entry, _ in prepared.tasks] with Interrupts() as interrupts: overrides = _overrides(sandbox, user_model, judge_model) @@ -278,7 +279,7 @@ def prepare_bundle( ``run_bundle`` raises before its first task runs, except for a running event loop, which it doesn't need.""" if sandbox: build_sandbox_provider(sandbox) - return _prepare(root, tasks, evals, sandbox, id_root, None, played, _progress(on_progress)) + return _prepare(root, tasks, evals, sandbox, id_root, None, played, _progress(on_progress), own=False) def _write_bundle( @@ -289,7 +290,7 @@ def _write_bundle( say = _progress(on_progress) with namespace_routing(): plan, preflight = _planned(root, (), (), id_root, None, None) - _say_agents(plan, say) + _say_plan(plan, say, own=False, dry_run=False) return _written_with_infra(plan, preflight, say) @@ -330,7 +331,7 @@ def dry_run_bundle( say = _progress(on_progress) with namespace_routing(): plan, preflight = _planned(root, tasks, evals, id_root, sandbox, agent) - _say_agents(plan, say) + _say_plan(plan, say, own=_own(root, id_root), dry_run=True) materialization = materialize(plan, dry_run=True, on_write=lambda done: say(_written(plan, done))) runs = _to_run(plan) return DryRun(materialization, runs, _skipped(plan, runs, every=not tasks and not evals), preflight.infra, @@ -359,9 +360,9 @@ def _bundle_run(plan: Plan, materialization: Materialization, runs: tuple[TaskRu def _prepare(root: Path | str, tasks: Sequence[str], evals: Sequence[str], sandbox: str | None, id_root: str | None, - agent: str | None, played: bool, say: Callable[[str], None]) -> PreparedBundle: + agent: str | None, played: bool, say: Callable[[str], None], *, own: bool) -> PreparedBundle: plan, preflight = _planned(root, tasks, evals, id_root, sandbox, agent, played=played) - _say_agents(plan, say) + _say_plan(plan, say, own=own, dry_run=False) materialization = _written_with_infra(plan, preflight, say) to_run = tuple((entry, Task.get(entry.id, materialization.version_of("task", entry.id))) for entry in _to_run(plan)) return PreparedBundle(plan, materialization, to_run) @@ -406,8 +407,10 @@ def refuse_store_tasks(plan: Plan) -> None: raise BundleError(problems) -def _say_agents(plan: Plan, say: Callable[[str], None]) -> None: - """Say what each agent, env and artifact alias names, and what the solver deploys in place of the tasks' agents.""" +def _say_plan(plan: Plan, say: Callable[[str], None], *, own: bool, dry_run: bool) -> None: + """Say what each agent, env and artifact alias names, what the solver deploys in place of the tasks' agents, and + what plugin packages the bundle uses that its bundle.toml doesn't require, giving a folder of its ``own`` one + (``declare``).""" for use in (use for uses in plan.alias_uses.values() for use in uses.values()): say(_aliased(use)) if plan.agent is not None: @@ -415,6 +418,14 @@ def _say_agents(plan: Plan, say: Callable[[str], None]) -> None: solved = sum(entry.id in plan.agent.steps for entry in runs) say(f"--agent {plan.agent.name}: the solver deploys {plan.agent.agent_id}, in {solved} of {len(runs)} " f"task{'s' if len(runs) != 1 else ''}") + for line in declare(plan.bundle, own=own, dry_run=dry_run): + say(line) + + +def _own(root: Path | str, id_root: str | None) -> bool: + """Whether the bundle at ``root`` is a folder run by its path that a file can be created in, not one an installed + package provides.""" + return id_root is None and writable(Path(root)) def _solver_params(plan: Plan, materialization: Materialization) -> dict[str, dict[str, dict]]: diff --git a/src/agent_env/cli/_install_prompt.py b/src/agent_env/cli/_install_prompt.py new file mode 100644 index 000000000..cf562f844 --- /dev/null +++ b/src/agent_env/cli/_install_prompt.py @@ -0,0 +1,47 @@ +"""Offer to install what a bundle's bundle.toml requires when a command is refused for it in a terminal, then run the +command again.""" + +from __future__ import annotations + +import os +import shlex +import sys + +import click + +from agent_env.bundle.requires import UnmetRequirements +from agent_env.cli import _plugin_changes +from agent_env.cli._installers import InstallerError + +# Set for the command run again once the install worked, so it never asks twice. +_ASKED = "AGENT_ENV_INSTALLED_REQUIRES" + + +def offer_to_install(error: UnmetRequirements) -> None: + """When ``error`` refused a command run in a terminal, print it and offer to install what it requires with + ``agent-env plugin add``; once that works, run the same command again in a new process, which loads the plugins it + added. Without a terminal, or when the command was already run again, it returns having printed nothing, for the + caller to raise ``error`` as it would have.""" + if os.environ.get(_ASKED) or not _interactive(): + return + click.echo("\n".join([f"Error: {error}", *getattr(error, "__notes__", ())]), err=True) + if not click.confirm(f"Install {', '.join(error.requirements)} with agent-env plugin add?", default=False): + raise SystemExit(1) + try: + code = _plugin_changes.add(list(error.requirements), installer=None, index_url=None, dry_run=False, yes=True, + keep=False, tools=_plugin_changes.Tools()) + except InstallerError as exc: + raise click.ClickException(str(exc)) from exc + if code: + raise SystemExit(code) + argv = [sys.executable, *sys.orig_argv[1:]] + click.echo(f"Running it again: {shlex.join(['agent-env', *sys.argv[1:]])}") + os.environ[_ASKED] = "1" + sys.stdout.flush() + sys.stderr.flush() + os.execv(sys.executable, argv) + + +def _interactive() -> bool: + """Whether a person at a terminal can answer: both stdin and stdout are one.""" + return sys.stdin.isatty() and sys.stdout.isatty() diff --git a/src/agent_env/cli/bundle.py b/src/agent_env/cli/bundle.py index 79e6adaad..5285f78c4 100644 --- a/src/agent_env/cli/bundle.py +++ b/src/agent_env/cli/bundle.py @@ -9,6 +9,8 @@ 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.bundle.requires import UnmetRequirements +from agent_env.cli._install_prompt import offer_to_install from agent_env.cli.utils import interruptible from agent_env.entity.scopes import FieldScope @@ -87,17 +89,27 @@ def register(folder: Path, tasks: tuple[str, ...], evals: tuple[str, ...], prefi 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.""" + registration. The plugin packages the bundle's bundle.toml requires must be installed, as for a run; a folder + with no bundle.toml that uses plugin packages is given one. 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}") click.echo("Checking:") with interruptible(): - planned = register_bundle(folder, dry_run=True, on_progress=click.echo, **options) + try: + planned = register_bundle(folder, dry_run=True, write_requires=not dry_run, on_progress=click.echo, + **options) + except UnmetRequirements as e: + if not dry_run: + offer_to_install(e) + raise for line in planned.floating: click.echo(f" floating, since its field takes no version: {line}") + for line in planned.requires: + click.echo(f" {line}") if dry_run: click.echo("Nothing was written: --dry-run.") return diff --git a/src/agent_env/cli/run.py b/src/agent_env/cli/run.py index 9a0dbd0d6..4fd25a688 100644 --- a/src/agent_env/cli/run.py +++ b/src/agent_env/cli/run.py @@ -14,6 +14,8 @@ dry_run_bundle, run_bundle) from agent_env.bundle.installed import (InstalledBundle, checked, find_bundle, installed_bundles, locate_bundle, path_like, run_name) +from agent_env.bundle.requires import UnmetRequirements +from agent_env.cli._install_prompt import offer_to_install from agent_env.plugins._bundles import ALIAS_TABLES, installed_aliases from agent_env.plugins._discovery import discovery_error from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox @@ -55,6 +57,10 @@ def run(ctx: click.Context, bundle: str | None, tasks: tuple[str, ...], evals: t BUNDLE is a folder, or the name of a bundle an installed package provides (PACKAGE/NAME when several packages provide that name). Without BUNDLE, it lists the installed bundles. + A bundle's bundle.toml names the plugin packages it requires, and a run refuses to start without them; in a + terminal it offers to install them with agent-env plugin add. A folder with no bundle.toml that uses plugin + packages is given one, which a dry run only says it would write. + With no --task or --eval, it runs every eval or, in a bundle without evals, every task. Each task runs once, at most four at a time, and the run exits 1 if any of them failed. With --verbose, a run that raised prints its traceback. --model, --user-model and --judge-model choose the models for this run only, @@ -91,6 +97,8 @@ def run(ctx: click.Context, bundle: str | None, tasks: tuple[str, ...], evals: t id_root=id_root, keep=keep) except BundleError as e: _note_an_installed_namesake(e, bundle, root, id_root) + if isinstance(e, UnmetRequirements) and not dry_run: + offer_to_install(e) raise except RunInterrupted as stop: with Interrupts(): diff --git a/tst/unit/bundle/importer_test.py b/tst/unit/bundle/importer_test.py index e9ce9ba7c..59f86db25 100644 --- a/tst/unit/bundle/importer_test.py +++ b/tst/unit/bundle/importer_test.py @@ -15,6 +15,7 @@ from agent_env.artifact import FileArtifact from agent_env.bundle import BundleError, parse_bundle from agent_env.bundle import importer as importer_module +from agent_env.bundle import requires as requires_module from agent_env.bundle.importer import ImportedEval, import_eval, import_task from agent_env.bundle.installed import InstalledBundle from agent_env.bundle.materialize import materialize @@ -622,8 +623,9 @@ def test_what_only_a_plugin_or_a_config_file_registers_is_named_as_a_need_by_wha monkeypatch.setattr(Config, "artifact_registry", lambda self: artifacts) acme = Distribution("agentenv-acme", "1.0", ( Contribution(TASK_STEPS, _Undeclared.type, f"{_Undeclared.__module__}:{_Undeclared.__qualname__}", "active"),)) - monkeypatch.setattr(importer_module, "inventory", lambda load: Inventory( + monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( distributions=(acme,), group_errors={}, discovery_errors={}, config_path=None, config_error=None, loaded=load)) + monkeypatch.setattr(requires_module, "installed", lambda name: "1.0" if name == "agentenv-acme" else None) (store / "acme.json").write_text("{}") _AcmeFile.put("acme-data", description="acme rows", file_path=str(store / "acme.json")) _task("acme", [{"id": "env", "type": "deploy_env", "env_id": "crm-env"}, @@ -641,6 +643,7 @@ def test_what_only_a_plugin_or_a_config_file_registers_is_named_as_a_need_by_wha "configuration: acme_file_import_test (artifact type), at tasks/acme.json: step 'load': artifacts[0].id", ]) assert "## Needs beyond core" in (imported.root / "README.md").read_text() + assert parse_bundle(imported.root).requires == ("agentenv-acme>=1.0",) def test_a_plugins_type_a_config_file_replaces_is_credited_to_configuration(store, monkeypatch): @@ -648,7 +651,7 @@ def test_a_plugins_type_a_config_file_replaces_is_credited_to_configuration(stor monkeypatch.setattr(Config, "task_step_registry", lambda self: steps) replaced = Distribution("agentenv-acme", "1.0", ( Contribution(TASK_STEPS, _Undeclared.type, f"{_AcmeFile.__module__}:{_AcmeFile.__qualname__}", "replaced"),)) - monkeypatch.setattr(importer_module, "inventory", lambda load: Inventory( + monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( distributions=(replaced,), group_errors={}, discovery_errors={}, config_path=None, config_error=None, loaded=load)) _task("acme", [{"id": "p", "type": _Undeclared.type}]) @@ -666,10 +669,10 @@ def test_a_skipped_plugins_type_a_config_file_supplies_is_credited_to_configurat monkeypatch.syspath_prepend(str(site)) broken = Distribution("agentenv-acme", "1.0", ( Contribution(TASK_STEPS, _Undeclared.type, "acme_broken_import_test:Step", "unloaded"),)) - monkeypatch.setattr(importer_module, "inventory", lambda load: Inventory( + monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( distributions=(broken,), group_errors={}, discovery_errors={}, config_path=None, config_error=None, loaded=load)) - monkeypatch.setattr(importer_module, "load_failures", lambda: {TASK_STEPS: {_Undeclared.type: "failed to load"}}) + monkeypatch.setattr(requires_module, "load_failures", lambda: {TASK_STEPS: {_Undeclared.type: "failed to load"}}) _task("acme", [{"id": "p", "type": _Undeclared.type}]) assert [need.by for need in import_task("acme", store / "acme").needs] == ["configuration"] diff --git a/tst/unit/bundle/parse_test.py b/tst/unit/bundle/parse_test.py index 8b339b468..d203d32b7 100644 --- a/tst/unit/bundle/parse_test.py +++ b/tst/unit/bundle/parse_test.py @@ -222,10 +222,46 @@ def test_not_a_bundle(self, tmp_path): (problem,) = problems(root) assert "not a bundle" in problem - @pytest.mark.parametrize("name", ["bundle.toml", "Bundle.TOML"]) - def test_bundle_toml_is_reserved(self, tmp_path, name): - root = build(tmp_path / "b", {name: "", "tasks/t.json": "[]"}) - assert problems(root)[0].startswith(f"{name}: reserved") + def test_no_bundle_toml_requires_nothing_it_knows_of(self, tmp_path): + assert parse_bundle(build(tmp_path / "b", {"tasks/t.json": "[]"})).requires is None + + @pytest.mark.parametrize("text, requires", [ + ("", ()), + ("requires = []\n", ()), + ('requires = ["agentenv-acme>=1.2", "agentenv-win; sys_platform == \'win32\'"]\n', + ("agentenv-acme>=1.2", "agentenv-win; sys_platform == 'win32'")), + ]) + def test_bundle_toml_lists_what_it_requires(self, tmp_path, text, requires): + root = build(tmp_path / "b", {"bundle.toml": text, "tasks/t.json": "[]"}) + assert parse_bundle(root).requires == requires + + @pytest.mark.parametrize("text, expected", [ + ("requires = [", "bundle.toml: isn't valid TOML: "), + ('requires = "agentenv-acme"\n', 'bundle.toml: requires must be a list of pip requirements, such as ' + '["agentenv-browser>=1.2"]'), + ("requires = [1]\n", "bundle.toml: requires must be a list of pip requirements"), + ('requires = []\nplugins = []\n', "bundle.toml: 'plugins' isn't a key it takes; it takes requires"), + ('requires = ["agentenv acme"]\n', "bundle.toml: requires: 'agentenv acme' isn't a pip requirement: "), + ('requires = ["acme @ https://example.com/acme.whl"]\n', + "bundle.toml: requires: 'acme @ https://example.com/acme.whl' names a URL; name the package and the " + "versions it needs"), + ]) + def test_a_bundle_toml_it_cant_use_is_a_problem(self, tmp_path, text, expected): + root = build(tmp_path / "b", {"bundle.toml": text, "tasks/t.json": "[]"}) + assert problems(root)[0].startswith(expected), problems(root) + + def test_bundle_toml_that_isnt_utf8_is_a_problem(self, tmp_path): + root = build(tmp_path / "b", {"bundle.toml": b"requires = ['\xff']\n", "tasks/t.json": "[]"}) + assert problems(root)[0].startswith("bundle.toml: isn't valid TOML: ") + + def test_bundle_toml_must_be_a_file(self, tmp_path): + root = build(tmp_path / "b", {"bundle.toml/": None, "tasks/t.json": "[]"}) + assert problems(root) == ("bundle.toml: must be a file, not a folder",) + + def test_bundle_toml_near_miss(self, tmp_path): + root = build(tmp_path / "b", {"Bundle.TOML": "", "tasks/t.json": "[]"}) + assert problems(root) == ("Bundle.TOML: spell it bundle.toml, the file a bundle declares the plugins it needs " + "in",) def test_kind_directory_near_miss(self, tmp_path): root = build(tmp_path / "b", {"Envs/tickets/Dockerfile": "FROM x"}) diff --git a/tst/unit/bundle/requires_test.py b/tst/unit/bundle/requires_test.py new file mode 100644 index 000000000..6d26f291e --- /dev/null +++ b/tst/unit/bundle/requires_test.py @@ -0,0 +1,443 @@ +"""A bundle's bundle.toml: what it requires is checked before the bundle resolves, a command refused for one this machine +doesn't meet names the command that installs it, and a bundle without one is given one listing the plugins it uses.""" + +import json +import os +import sys +import threading +import tomllib + +import pytest +from click.testing import CliRunner + +from agent_env.bundle import BundleError, dry_run_bundle, parse_bundle, prepare_bundle, run_bundle +from agent_env.bundle import register as register_module +from agent_env.bundle import requires as requires_module +from agent_env.bundle.register import register_bundle +from agent_env.bundle.requires import ( + Provider, Providers, UnmetRequirements, _create, declare, install_command, unmet, unmet_problems, +) +from agent_env.bundle.resolve import resolve_bundle +from agent_env.cli import _install_prompt, cli +from agent_env.cli._installers import InstallerError +from agent_env.config.runtime import Config +from agent_env.plugins import TASK_STEPS, Contribution, Distribution, Inventory +from agent_env.store.routing import namespace_routing +from agent_env.task_step.task_step import TaskStep +from tst.unit.bundle._support import layout + +ACME = "agentenv-acme" +WROTE = ('wrote bundle.toml: requires = ["agentenv-acme>=1.2.0"]; commit it so whoever you share the bundle with knows ' + "what to install") +MISSING = ("bundle.toml: requires 'agentenv-missing>=1', which isn't installed; install it with: agent-env plugin add " + "'agentenv-missing>=1'") + + +class _Acme(TaskStep): + """A step type the plugin agentenv-acme ships.""" + + type = "acme_requires_test" + entity_refs = () + + @classmethod + def from_dict(cls, data): + return cls(**cls._base_from_dict(data)) + + async def execute(self, context): + return context + + +class _Configured(_Acme): + """A step type only this machine's configuration registers.""" + + type = "configured_requires_test" + + +class _Replaced(_Acme): + """A step type a config file registers over the one agentenv-acme ships under the same name.""" + + type = "replaced_requires_test" + + +def _value(cls): + return f"{cls.__module__}:{cls.__qualname__}" + + +@pytest.fixture(autouse=True) +def acme(monkeypatch): + """agentenv-acme 1.2.0 installed, its step type registered as its entry point registers it, beside a step type only + configuration registers and one configuration registers over agentenv-acme's.""" + steps = {**Config().task_step_registry(), **{cls.type: cls for cls in (_Acme, _Configured, _Replaced)}} + monkeypatch.setattr(Config, "task_step_registry", lambda self: steps) + contributions = (Contribution(TASK_STEPS, _Acme.type, _value(_Acme), "active"), + Contribution(TASK_STEPS, _Replaced.type, _value(_Acme), "replaced")) + monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( + distributions=(Distribution(ACME, "1.2.0", contributions),), group_errors={}, discovery_errors={}, + config_path=None, config_error=None, loaded=load)) + monkeypatch.setattr(requires_module, "load_failures", dict) + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0"}.get) + + +def _bundle(root, *types, toml=None): + files = {"tasks/t.json": json.dumps([{"id": f"s{i}", "type": step_type} for i, step_type in enumerate(types)])} + if toml is not None: + files["bundle.toml"] = toml + return layout(root, files) + + +def _declared(root, *, own=True, dry_run=False): + return declare(resolve_bundle(parse_bundle(root)), own=own, dry_run=dry_run) + + +class TestProviders: + def test_what_core_ships_has_no_provider(self): + assert Providers().of(TASK_STEPS, "run_code", None) is None + + def test_a_plugins_type_is_provided_by_its_distribution_at_its_version(self): + assert Providers().of(TASK_STEPS, _Acme.type, _Acme) == Provider(ACME, "1.2.0") + assert Provider(ACME, "1.2.0").requirement() == "agentenv-acme>=1.2.0" + + @pytest.mark.parametrize("cls", [_Configured, _Replaced]) + def test_a_type_configuration_registers_is_provided_by_configuration(self, cls): + assert Providers().of(TASK_STEPS, cls.type, cls) == Provider(None, None) + assert Provider(None, None).requirement() is None + + def test_a_plugin_that_failed_to_load_provides_nothing(self, monkeypatch): + monkeypatch.setattr(requires_module, "load_failures", lambda: {TASK_STEPS: {_Acme.type: "failed to load"}}) + assert Providers().of(TASK_STEPS, _Acme.type, _Acme) == Provider(None, None) + + +class TestUnmet: + @pytest.mark.parametrize("line", ["agentenv-acme", "agentenv-acme>=1.0,<2", "AgentEnv_Acme>=1", + "agentenv-missing; sys_platform == 'no-such-platform'"]) + def test_a_requirement_met_here_or_that_doesnt_apply_is_met(self, line): + assert unmet([line]) == [] + + def test_a_package_not_installed_or_at_a_version_it_excludes_is_unmet(self): + assert unmet(["agentenv-missing", "agentenv-acme>=2"]) == [ + ("agentenv-missing", "which isn't installed"), ("agentenv-acme>=2", "and agentenv-acme 1.2.0 is installed")] + + def test_one_unmet_requirement_names_the_command_that_installs_it(self): + assert unmet_problems(["agentenv-missing>=1", "agentenv-acme"]) == ([MISSING], ["agentenv-missing>=1"]) + + def test_several_name_one_command_that_installs_them_all(self): + assert unmet_problems(["agentenv-missing>=1", "agentenv-acme>=2"]) == ([ + "bundle.toml: requires 'agentenv-missing>=1', which isn't installed", + "bundle.toml: requires 'agentenv-acme>=2', and agentenv-acme 1.2.0 is installed", + "bundle.toml: install what it requires with: agent-env plugin add 'agentenv-missing>=1' 'agentenv-acme>=2'", + ], ["agentenv-missing>=1", "agentenv-acme>=2"]) + + def test_the_command_quotes_what_a_shell_would_split(self): + assert install_command(["a; sys_platform == 'linux'"]) == \ + "agent-env plugin add 'a; sys_platform == '\"'\"'linux'\"'\"''" + + def test_a_bundle_requiring_what_isnt_here_is_refused_before_it_resolves_with_every_other_problem(self, tmp_path): + root = _bundle(tmp_path / "b", "acme_plugin_not_here", toml='requires = ["agentenv-missing>=1"]\n') + + with pytest.raises(UnmetRequirements) as caught: + resolve_bundle(parse_bundle(root)) + + assert caught.value.requirements == ("agentenv-missing>=1",) + assert MISSING in caught.value.problems + assert any("acme_plugin_not_here" in problem for problem in caught.value.problems), caught.value.problems + + def test_a_bundle_whose_requirements_are_met_resolves(self, tmp_path): + resolve_bundle(parse_bundle(_bundle(tmp_path / "b", _Acme.type, toml=f'requires = ["{ACME}>=1"]\n'))) + + +class TestDeclare: + def test_a_bundle_without_one_is_given_a_bundle_toml_listing_the_plugins_it_uses(self, tmp_path): + root = _bundle(tmp_path / "b", _Acme.type, _Acme.type, "run_code") + + assert _declared(root) == [WROTE] + assert tomllib.loads((root / "bundle.toml").read_text()) == {"requires": ["agentenv-acme>=1.2.0"]} + assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) + assert _declared(root) == [] + + def test_a_dry_run_says_what_it_would_write(self, tmp_path): + root = _bundle(tmp_path / "b", _Acme.type) + + assert _declared(root, dry_run=True) == ['would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]'] + assert not (root / "bundle.toml").exists() + + def test_a_bundle_using_only_what_core_ships_is_given_none(self, tmp_path): + root = _bundle(tmp_path / "b", "run_code") + + assert _declared(root) == [] + assert not (root / "bundle.toml").exists() + + @pytest.mark.parametrize("toml, requires", [ + ("requires = []\n", ["agentenv-acme>=1.2.0"]), + ('requires = ["agentenv-other"]\n', ["agentenv-acme>=1.2.0", "agentenv-other"]), + ], ids=["opted-out", "requires-another"]) + def test_an_existing_bundle_toml_is_never_edited_but_told_what_to_declare(self, tmp_path, monkeypatch, toml, + requires): + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-other": "1"}.get) + root = _bundle(tmp_path / "b", _Acme.type, toml=toml) + + assert _declared(root) == [ + "this bundle uses agentenv-acme 1.2.0 (tasks/t.json: step 's0'), which bundle.toml doesn't require", + f"declare them in bundle.toml, so whoever it's shared with knows what to install: requires = " + f"{json.dumps(requires)}"] + assert (root / "bundle.toml").read_text() == toml + + def test_a_folder_not_its_own_is_told_what_to_declare(self, tmp_path): + root = _bundle(tmp_path / "b", _Acme.type, _Acme.type) + + assert _declared(root, own=False) == [ + "this bundle uses agentenv-acme 1.2.0 (tasks/t.json: step 's0', and 1 more), which bundle.toml doesn't " + "require", + 'declare them in bundle.toml, so whoever it\'s shared with knows what to install: requires = ' + '["agentenv-acme>=1.2.0"]'] + assert not (root / "bundle.toml").exists() + + def test_a_type_only_configuration_registers_is_named_and_never_required(self, tmp_path): + root = _bundle(tmp_path / "b", _Configured.type, _Acme.type, _Replaced.type) + + assert _declared(root) == [ + WROTE, + "this bundle uses the step type 'configured_requires_test' and the step type 'replaced_requires_test' " + "(tasks/t.json: step 's0', and 1 more), which this machine's configuration registers; whoever runs it " + "needs that configuration too"] + assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) + + def test_a_bundle_toml_that_appears_meanwhile_is_kept_and_told_what_to_declare(self, tmp_path, monkeypatch): + root = _bundle(tmp_path / "b", _Acme.type) + resolved = resolve_bundle(parse_bundle(root)) + (root / "bundle.toml").write_text("requires = []\n") + + assert declare(resolved, own=True, dry_run=False)[0].startswith("this bundle uses agentenv-acme 1.2.0") + assert (root / "bundle.toml").read_text() == "requires = []\n" + + +class TestCreate: + def test_it_never_writes_over_a_file_and_leaves_nothing_staged(self, tmp_path): + path = tmp_path / "bundle.toml" + path.write_text("mine\n") + + assert _create(path, "theirs\n") is False + assert path.read_text() == "mine\n" + assert os.listdir(tmp_path) == ["bundle.toml"] + + def test_of_runs_racing_to_create_it_one_writes_it_whole(self, tmp_path): + path = tmp_path / "bundle.toml" + texts = [f"requires = [{i!r}]\n" * 200 for i in range(16)] + start = threading.Barrier(len(texts)) + created = [] + + def create(text): + start.wait() + created.append(_create(path, text)) + + threads = [threading.Thread(target=create, args=(text,)) for text in texts] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + assert created.count(True) == 1 + assert path.read_text() in texts + assert os.listdir(tmp_path) == ["bundle.toml"] + + def test_without_links_it_creates_it_in_place_still_never_over_another(self, tmp_path, monkeypatch): + def no_links(src, dst): + raise PermissionError("links aren't supported here") + + monkeypatch.setattr(requires_module.os, "link", no_links) + path = tmp_path / "bundle.toml" + + assert _create(path, "first\n") is True + assert _create(path, "second\n") is False + assert path.read_text() == "first\n" + assert os.listdir(tmp_path) == ["bundle.toml"] + + @pytest.mark.skipif(sys.platform == "win32" or os.geteuid() == 0, reason="a folder root can write to anyway") + def test_a_folder_it_cant_write_to_is_left_as_it_is(self, tmp_path): + tmp_path.chmod(0o555) + try: + assert _create(tmp_path / "bundle.toml", "x\n") is False + assert os.listdir(tmp_path) == [] + finally: + tmp_path.chmod(0o755) + + +@pytest.fixture +def stores(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + return tmp_path + + +class TestCommands: + def test_a_run_writes_it_and_a_dry_run_or_a_service_run_doesnt(self, stores): + root = _bundle(stores / "b", _Acme.type) + dry, prepared, ran = [], [], [] + + dry_run_bundle(root, on_progress=dry.append) + with namespace_routing(): + prepare_bundle(root, on_progress=prepared.append) + assert not (root / "bundle.toml").exists() + run_bundle(root, on_progress=ran.append) + + assert 'would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]' in dry + assert prepared[0].startswith("this bundle uses agentenv-acme 1.2.0") and WROTE not in prepared + assert WROTE in ran + assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) + + @pytest.mark.skipif(sys.platform == "win32" or os.geteuid() == 0, reason="a folder root can write to anyway") + def test_a_run_of_a_folder_it_cant_write_to_says_what_to_declare(self, stores): + root = _bundle(stores / "b", _Acme.type) + lines = [] + root.chmod(0o555) + try: + run_bundle(root, on_progress=lines.append) + finally: + root.chmod(0o755) + + assert "this bundle uses agentenv-acme 1.2.0 (tasks/t.json: step 's0'), which bundle.toml doesn't require" in lines + assert not (root / "bundle.toml").exists() + + def test_a_registration_writes_it_unless_a_dry_run(self, stores, monkeypatch): + monkeypatch.setattr(register_module, "_git", lambda root, *args: None) + root = _bundle(stores / "b", _Acme.type) + + dry = register_bundle(root, dry_run=True).requires + assert not (root / "bundle.toml").exists() + registered = register_bundle(root, dry_run=True, write_requires=True).requires + + assert dry == ('would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]',) + assert registered == (WROTE,) + assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) + assert register_bundle(root).requires == () + + def test_the_cli_registration_writes_it_unless_a_dry_run_and_says_so_once(self, stores, monkeypatch): + monkeypatch.setattr(register_module, "_git", lambda root, *args: None) + root = _bundle(stores / "b", _Acme.type) + + dry = CliRunner().invoke(cli, ["bundle", "register", str(root), "--dry-run"]) + assert not (root / "bundle.toml").exists() + done = CliRunner().invoke(cli, ["bundle", "register", str(root)]) + + assert dry.exit_code == 0 and done.exit_code == 0, dry.output + done.output + assert dry.output.count(' would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]\n') == 1, dry.output + assert done.output.count(f" {WROTE}\n") == 1, done.output + assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) + + def test_the_cli_registration_says_what_to_declare_once(self, stores, monkeypatch): + monkeypatch.setattr(register_module, "_git", lambda root, *args: None) + root = _bundle(stores / "b", _Acme.type, toml="requires = []\n") + + result = CliRunner().invoke(cli, ["bundle", "register", str(root)]) + + assert result.exit_code == 0, result.output + assert result.output.count("this bundle uses agentenv-acme 1.2.0") == 1, result.output + + def test_the_cli_refuses_a_run_whose_requirements_arent_met_without_a_terminal(self, stores, monkeypatch): + added = [] + monkeypatch.setattr(_install_prompt._plugin_changes, "add", lambda *a, **k: added.append(a)) + root = _bundle(stores / "b", _Acme.type, toml='requires = ["agentenv-missing>=1"]\n') + + result = CliRunner().invoke(cli, ["run", str(root)]) + + assert result.exit_code == 1 + assert f"Error: {MISSING}" in result.output + assert "Install" not in result.output and added == [] + + +@pytest.fixture +def terminal(stores, monkeypatch): + """A person at a terminal, an install that records what it's asked for, and a re-exec that stops the test.""" + calls = {"added": [], "exec": []} + + def add(specs, **kwargs): + calls["added"].append((specs, kwargs["yes"])) + return calls.get("code", 0) + + def execv(path, argv): + calls["exec"].append((path, argv, os.environ.get(_install_prompt._ASKED))) + raise SystemExit(0) + + monkeypatch.setattr(_install_prompt, "_interactive", lambda: True) + monkeypatch.setattr(_install_prompt._plugin_changes, "add", add) + monkeypatch.setattr(_install_prompt.os, "execv", execv) + monkeypatch.setenv(_install_prompt._ASKED, "") + root = _bundle(stores / "b", _Acme.type, toml='requires = ["agentenv-missing>=1"]\n') + monkeypatch.setattr(sys, "argv", ["agent-env", "run", str(root)]) + return root, calls + + +class TestInstallPrompt: + def test_yes_installs_it_and_runs_the_command_again(self, terminal): + root, calls = terminal + + result = CliRunner().invoke(cli, ["run", str(root)], input="y\n") + + assert result.exit_code == 0, result.output + assert result.output.count("Error:") == 1 + assert f"Error: {MISSING}" in result.output + assert "Install agentenv-missing>=1 with agent-env plugin add? [y/N]: y" in result.output + assert f"Running it again: agent-env run {root}" in result.output + assert calls["added"] == [(["agentenv-missing>=1"], True)] + assert calls["exec"] == [(sys.executable, [sys.executable, *sys.orig_argv[1:]], "1")] + + def test_no_installs_nothing_and_exits_1(self, terminal): + root, calls = terminal + + result = CliRunner().invoke(cli, ["run", str(root)], input="n\n") + + assert result.exit_code == 1 + assert result.output.count("Error:") == 1 + assert calls["added"] == [] and calls["exec"] == [] + + def test_a_command_run_again_after_an_install_never_asks_again(self, terminal, monkeypatch): + root, calls = terminal + monkeypatch.setenv(_install_prompt._ASKED, "1") + + result = CliRunner().invoke(cli, ["run", str(root)], input="y\n") + + assert result.exit_code == 1 + assert f"Error: {MISSING}" in result.output and "Install" not in result.output + assert calls["added"] == [] + + def test_a_dry_run_never_asks(self, terminal): + root, calls = terminal + + result = CliRunner().invoke(cli, ["run", str(root), "--dry-run"], input="y\n") + + assert result.exit_code == 1 + assert "Install" not in result.output and calls["added"] == [] + + def test_an_install_that_fails_stops_without_running_it_again(self, terminal, monkeypatch): + root, calls = terminal + + def refused(specs, **kwargs): + raise InstallerError("no installer could reach the index") + + monkeypatch.setattr(_install_prompt._plugin_changes, "add", refused) + + result = CliRunner().invoke(cli, ["run", str(root)], input="y\n") + + assert result.exit_code == 1 + assert "Error: no installer could reach the index" in result.output + assert calls["exec"] == [] + + def test_an_install_that_exits_non_zero_exits_with_its_code(self, terminal): + root, calls = terminal + calls["code"] = 2 + + result = CliRunner().invoke(cli, ["run", str(root)], input="y\n") + + assert result.exit_code == 2 + assert calls["exec"] == [] + + def test_a_registration_refused_for_it_asks_too(self, terminal): + root, calls = terminal + + result = CliRunner().invoke(cli, ["bundle", "register", str(root)], input="y\n") + + assert result.exit_code == 0, result.output + assert calls["added"] == [(["agentenv-missing>=1"], True)] + assert len(calls["exec"]) == 1 + + +def test_unmet_requirements_are_a_bundle_error(): + assert issubclass(UnmetRequirements, BundleError) diff --git a/tst/unit/bundle/run_test.py b/tst/unit/bundle/run_test.py index d8bca1b7d..ef732f8c7 100644 --- a/tst/unit/bundle/run_test.py +++ b/tst/unit/bundle/run_test.py @@ -111,6 +111,15 @@ def to_dict(self): return {**super().to_dict(), "a2a_agent_id": self.a2a_agent_id} +# The hint a run gives for the step types these tests register through configuration, as a config file would. +CONFIGURED = ("this bundle uses the step type 'scored_run_test' and the step type 'failing_run_test' (tasks/a.json: " + "step 'check', and 3 more), which this machine's configuration registers; whoever runs it needs that " + "configuration too") +CONFIGURED_WITH_AGENT = ("this bundle uses the step type 'scored_run_test', the step type 'names_agent_run_test' and " + "the step type 'failing_run_test' (tasks/a.json: step 'check', and 4 more), which this " + "machine's configuration registers; whoever runs it needs that configuration too") + + def _scored(score): return json.dumps([{"id": "check", "type": "scored_run_test", "verifier_id": "v", "score": score}]) @@ -329,6 +338,7 @@ def test_progress_is_reported_and_a_progress_callback_that_raises_stops_without_ run_bundle(bundle_dir, tasks=["a"], on_progress=lines.append) assert [re.sub(r"\d+\.\ds", "", line) for line in lines] == [ + CONFIGURED, "tasks/a.json: v1 (new)", "[tasks/a.json] step 1/1 check (scored_run_test)", "[tasks/a.json] step 1/1 check done in ", @@ -342,7 +352,7 @@ def broken(line): raise BrokenPipeError assert run_bundle(bundle_dir, tasks=["a"], on_progress=broken).runs[0].outcome is Outcome.PASSED - assert calls == ["tasks/a.json: v1, unchanged"] + assert calls == [CONFIGURED] def test_a_run_says_before_it_builds_an_image_and_names_the_image_apart_from_its_agent(bundle_dir, docker, @@ -356,7 +366,8 @@ def test_a_run_says_before_it_builds_an_image_and_names_the_image_apart_from_its run_bundle(bundle_dir, tasks=["agent"], on_progress=lines.append) - assert lines[:4] == [ + assert lines[:5] == [ + CONFIGURED_WITH_AGENT, "agents/solver (Dockerfile image): building with docker, which can take minutes", "agents/solver (Dockerfile image): v1 (new)", "agents/solver: v1 (new)", @@ -541,7 +552,8 @@ def test_the_cli_dry_run_prints_what_the_run_would_write_and_run_and_exits_0(bun assert result.exit_code == 0, result.output assert result.output == ( DRY_RUN - + "artifacts/script: v1 (new)\n" + + f"{CONFIGURED}\n" + "artifacts/script: v1 (new)\n" "tasks/a.json: v1 (new)\n" "tasks/b.json: v1 (new)\n" "tasks/c.json: v1 (new)\n" @@ -583,7 +595,8 @@ def test_the_cli_dry_run_reads_another_bundles_entity_and_says_why_an_agent_pinn assert result.exit_code == 0, result.output assert result.output.startswith( DRY_RUN - + f"agents/solver: v2 (artifact {image} has a new version in the store (v1 → v2))\n" + + f"{CONFIGURED}\n" + f"agents/solver: v2 (artifact {image} has a new version in the store (v1 → v2))\n" "tasks/agent.json: v1, unchanged\n" "\n" "Store refs:\n" @@ -601,7 +614,8 @@ def test_the_cli_dry_run_lists_an_image_it_would_write_and_builds_nothing(bundle assert result.exit_code == 0, result.output assert result.output == ( DRY_RUN - + "agents/solver (Dockerfile image): v1 (new)\n" + + f"{CONFIGURED_WITH_AGENT}\n" + "agents/solver (Dockerfile image): v1 (new)\n" "agents/solver: v1 (new)\n" "tasks/agent.json: v1 (new)\n" "\n" diff --git a/tst/unit/config/test_explain_and_sources.py b/tst/unit/config/test_explain_and_sources.py index a71f8df76..676e016fb 100644 --- a/tst/unit/config/test_explain_and_sources.py +++ b/tst/unit/config/test_explain_and_sources.py @@ -162,7 +162,8 @@ def test_sources_says_how_the_file_was_found(config_file, tmp_path, monkeypatch) # Read for identity (USER), or set from [sandbox.providers.sail_vm.config] for the Sail SDK to read # (SAIL_*), never configuration agent-env takes, so none is a layer. -_NOT_CONFIGURATION = {"USER", "SAIL_API_KEY", "SAIL_RUNTIME_THREADS"} +# AGENT_ENV_INSTALLED_REQUIRES marks a command run again after installing what a bundle requires, so it never asks twice. +_NOT_CONFIGURATION = {"USER", "SAIL_API_KEY", "SAIL_RUNTIME_THREADS", "AGENT_ENV_INSTALLED_REQUIRES"} def _env_vars_read_by(root): From 21694b43365d9e3db84f4a2505603f639d650b0b Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 15:23:54 -0700 Subject: [PATCH 2/6] fix(bundle): a bundle.toml that can't be written whole is removed, not left empty On a full disk the in-place fallback could leave an empty bundle.toml, which reads as requires = [] and silently opts the bundle out. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/requires.py | 31 +++++++++++++++++++------------ tst/unit/bundle/requires_test.py | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/agent_env/bundle/requires.py b/src/agent_env/bundle/requires.py index 50b90d5ea..97e9814c9 100644 --- a/src/agent_env/bundle/requires.py +++ b/src/agent_env/bundle/requires.py @@ -200,27 +200,34 @@ def _create(path: Path, text: str) -> bool: it under a name of its own, as the umask leaves new files, and linked into place, which fails if ``path`` exists.""" staged = path.with_name(f".{path.name}.{uuid.uuid4().hex}.tmp") try: - fd = os.open(staged, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o666) - except OSError: - return False - try: - with os.fdopen(fd, "w", encoding="utf-8") as file: - file.write(text) + if not _write_new(staged, text): + return False os.link(staged, path) return True except FileExistsError: return False except OSError: # a filesystem without links: create it in place, still never over another's - try: - with open(path, "x", encoding="utf-8") as file: - file.write(text) - return True - except OSError: - return False + return _write_new(path, text) finally: staged.unlink(missing_ok=True) +def _write_new(path: Path, text: str) -> bool: + """Create ``path`` holding ``text``, unless it exists; whether it was written. One that can't be written whole, as + on a full disk, is removed again, so no part of it is left to read.""" + try: + file = open(path, "x", encoding="utf-8") + except OSError: + return False + try: + with file: + file.write(text) + return True + except OSError: + path.unlink(missing_ok=True) + return False + + def _listing(found: Sequence[Use]) -> str: """Each distinct type ``found`` names, once: ``the step type 'a' and the env type 'b'``.""" named = list(dict.fromkeys(f"the {use.what} {use.name!r}" for use in found)) diff --git a/tst/unit/bundle/requires_test.py b/tst/unit/bundle/requires_test.py index 6d26f291e..2e9d31179 100644 --- a/tst/unit/bundle/requires_test.py +++ b/tst/unit/bundle/requires_test.py @@ -1,6 +1,7 @@ """A bundle's bundle.toml: what it requires is checked before the bundle resolves, a command refused for one this machine doesn't meet names the command that installs it, and a bundle without one is given one listing the plugins it uses.""" +import errno import json import os import sys @@ -251,6 +252,22 @@ def no_links(src, dst): assert path.read_text() == "first\n" assert os.listdir(tmp_path) == ["bundle.toml"] + @pytest.mark.parametrize("links", [True, False], ids=["staged", "in-place"]) + def test_a_file_it_cant_write_whole_is_never_left_behind(self, tmp_path, monkeypatch, links): + real_open = open + + def full(path, mode="r", **kwargs): + file = real_open(path, mode, **kwargs) + file.write = lambda text: (_ for _ in ()).throw(OSError(errno.ENOSPC, "No space left on device")) + return file + + monkeypatch.setattr(requires_module, "open", full, raising=False) + if not links: + monkeypatch.setattr(requires_module.os, "link", lambda src, dst: (_ for _ in ()).throw(PermissionError())) + + assert _create(tmp_path / "bundle.toml", "requires = []\n") is False + assert os.listdir(tmp_path) == [] + @pytest.mark.skipif(sys.platform == "win32" or os.geteuid() == 0, reason="a folder root can write to anyway") def test_a_folder_it_cant_write_to_is_left_as_it_is(self, tmp_path): tmp_path.chmod(0o555) From 1a0086ea922587e6332fb7fd78b990be4bfed207 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 15:30:02 -0700 Subject: [PATCH 3/6] fix(bundle): bundle.toml bounds drop local labels, extras are checked, and nothing is written without links A plugin installed at a local version (1.2.0+acme.1) is required at >=1.2.0, a bound pip takes. A requirement naming extras is met only when what those extras need is installed too. A filesystem without hard links gets the hint rather than a bundle.toml written in place, which another run could read half-written. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/requires.py | 85 ++++++++++++++++++++++---------- tst/unit/bundle/requires_test.py | 73 +++++++++++++++++++++------ 2 files changed, 119 insertions(+), 39 deletions(-) diff --git a/src/agent_env/bundle/requires.py b/src/agent_env/bundle/requires.py index 97e9814c9..abbd4a18d 100644 --- a/src/agent_env/bundle/requires.py +++ b/src/agent_env/bundle/requires.py @@ -11,11 +11,13 @@ import uuid from collections.abc import Iterable, Sequence from dataclasses import dataclass +from importlib.metadata import PackageNotFoundError, distribution from pathlib import Path from typing import TYPE_CHECKING -from packaging.requirements import Requirement +from packaging.requirements import InvalidRequirement, Requirement from packaging.utils import canonicalize_name +from packaging.version import InvalidVersion, Version from agent_env.artifact.registry import ARTIFACT_REGISTRY, canonical_type, get_artifact_registry from agent_env.env import registry as env_registry @@ -55,8 +57,14 @@ class Provider: version: str | None def requirement(self) -> str | None: - """The pip requirement for it, at least the version installed here; None for configuration.""" - return f"{self.distribution}>={self.version}" if self.distribution else None + """The pip requirement for it, at least the version installed here without a local label, which a bound can't + carry, or any version when that isn't one pip compares; None for configuration.""" + if not self.distribution: + return None + try: + return f"{self.distribution}>={Version(self.version).public}" + except (InvalidVersion, TypeError): + return self.distribution @dataclass(frozen=True) @@ -92,8 +100,8 @@ def of(self, group: str, name: str, cls: type | None) -> Provider | None: def unmet(requires: Iterable[str]) -> list[tuple[str, str]]: - """Each of ``requires`` that isn't met here, with why: its package isn't installed, or is at a version it excludes. - One whose marker is false here doesn't apply.""" + """Each of ``requires`` that isn't met here, with why: its package isn't installed, is at a version it excludes, or + is without what the extras it names need. One whose marker is false here doesn't apply.""" found = [] for line in requires: requirement = Requirement(line) @@ -102,11 +110,51 @@ def unmet(requires: Iterable[str]) -> list[tuple[str, str]]: have = installed(canonicalize_name(requirement.name)) if have is None: found.append((line, "which isn't installed")) - elif not requirement.specifier.contains(have, prereleases=True): + elif not _allows(requirement, have): found.append((line, f"and {requirement.name} {have} is installed")) + elif missing := _missing_for_extras(requirement, set()): + found.append((line, f"and {requirement.name} {have} is installed without {', '.join(missing)}, which its " + f"extras need")) return found +def _allows(requirement: Requirement, version: str) -> bool: + """Whether ``requirement`` admits ``version``; one pip can't compare it admits only when it names no versions.""" + try: + return requirement.specifier.contains(version, prereleases=True) + except InvalidVersion: + return not requirement.specifier + + +def _missing_for_extras(requirement: Requirement, seen: set[tuple[str, str]]) -> list[str]: + """What the extras ``requirement`` names need of its installed package that isn't installed, or not at a version + it allows, following an extra that names another of the package's own.""" + name = canonicalize_name(requirement.name) + missing = [] + for extra in sorted(requirement.extras): + if (name, extra) in seen: + continue + seen.add((name, extra)) + try: + lines = distribution(name).requires or () + except PackageNotFoundError: + return [] + for line in lines: + try: + needed = Requirement(line) + except InvalidRequirement: + continue + if needed.marker is None or not needed.marker.evaluate({"extra": extra}) \ + or needed.marker.evaluate({"extra": ""}): + continue + have = installed(canonicalize_name(needed.name)) + if have is None or not _allows(needed, have): + missing.append(f"{needed.name}{needed.specifier}") + else: + missing.extend(_missing_for_extras(needed, seen)) + return list(dict.fromkeys(missing)) + + def unmet_problems(requires: Iterable[str]) -> tuple[list[str], list[str]]: """The problems ``bundle.toml``'s unmet ``requires`` raise, naming the one command that installs them all, and the unmet requirements.""" @@ -197,35 +245,22 @@ def writable(root: Path) -> bool: def _create(path: Path, text: str) -> bool: """Write ``text`` to ``path`` whole, unless something is there by then; whether it was written. It's staged beside - it under a name of its own, as the umask leaves new files, and linked into place, which fails if ``path`` exists.""" + it under a name of its own, as the umask leaves new files, and linked into place, which fails if ``path`` exists; + on a filesystem without links, or a full one, nothing is written.""" staged = path.with_name(f".{path.name}.{uuid.uuid4().hex}.tmp") try: - if not _write_new(staged, text): - return False - os.link(staged, path) - return True - except FileExistsError: - return False - except OSError: # a filesystem without links: create it in place, still never over another's - return _write_new(path, text) - finally: - staged.unlink(missing_ok=True) - - -def _write_new(path: Path, text: str) -> bool: - """Create ``path`` holding ``text``, unless it exists; whether it was written. One that can't be written whole, as - on a full disk, is removed again, so no part of it is left to read.""" - try: - file = open(path, "x", encoding="utf-8") + file = open(staged, "x", encoding="utf-8") except OSError: return False try: with file: file.write(text) + os.link(staged, path) return True except OSError: - path.unlink(missing_ok=True) return False + finally: + staged.unlink(missing_ok=True) def _listing(found: Sequence[Use]) -> str: diff --git a/tst/unit/bundle/requires_test.py b/tst/unit/bundle/requires_test.py index 2e9d31179..1ed315f83 100644 --- a/tst/unit/bundle/requires_test.py +++ b/tst/unit/bundle/requires_test.py @@ -10,6 +10,7 @@ import pytest from click.testing import CliRunner +from packaging.requirements import Requirement from agent_env.bundle import BundleError, dry_run_bundle, parse_bundle, prepare_bundle, run_bundle from agent_env.bundle import register as register_module @@ -48,6 +49,13 @@ async def execute(self, context): return context +class _AcmeMetadata: + """agentenv-acme's installed requirements: a base one, an extra each, and an extra naming another of its own.""" + + requires = ["packaging", "playwright>=1.40; extra == 'browser'", "torch; extra == 'gpu'", + "agentenv-acme[gpu]; extra == 'all'"] + + class _Configured(_Acme): """A step type only this machine's configuration registers.""" @@ -60,6 +68,10 @@ class _Replaced(_Acme): type = "replaced_requires_test" +def _no_links(src, dst): + raise PermissionError("links aren't supported here") + + def _value(cls): return f"{cls.__module__}:{cls.__qualname__}" @@ -98,6 +110,15 @@ def test_a_plugins_type_is_provided_by_its_distribution_at_its_version(self): assert Providers().of(TASK_STEPS, _Acme.type, _Acme) == Provider(ACME, "1.2.0") assert Provider(ACME, "1.2.0").requirement() == "agentenv-acme>=1.2.0" + @pytest.mark.parametrize("version, requirement", [ + ("1.2.0+acme.1", "agentenv-acme>=1.2.0"), + ("1!2.0.post1", "agentenv-acme>=1!2.0.post1"), + ("not a version", "agentenv-acme"), + ], ids=["local-label", "epoch-post", "legacy"]) + def test_a_requirement_is_one_pip_takes_whatever_the_installed_version(self, version, requirement): + assert Provider(ACME, version).requirement() == requirement + assert str(Requirement(requirement)) == requirement + @pytest.mark.parametrize("cls", [_Configured, _Replaced]) def test_a_type_configuration_registers_is_provided_by_configuration(self, cls): assert Providers().of(TASK_STEPS, cls.type, cls) == Provider(None, None) @@ -118,6 +139,31 @@ def test_a_package_not_installed_or_at_a_version_it_excludes_is_unmet(self): assert unmet(["agentenv-missing", "agentenv-acme>=2"]) == [ ("agentenv-missing", "which isn't installed"), ("agentenv-acme>=2", "and agentenv-acme 1.2.0 is installed")] + def test_a_version_pip_cant_compare_meets_only_a_requirement_naming_no_versions(self, monkeypatch): + monkeypatch.setattr(requires_module, "installed", {ACME: "not a version"}.get) + + assert unmet(["agentenv-acme", "agentenv-acme>=1"]) == [ + ("agentenv-acme>=1", "and agentenv-acme not a version is installed")] + + @pytest.mark.parametrize("line, missing", [ + ("agentenv-acme[browser]>=1", "playwright>=1.40"), + ("agentenv-acme[all]", "torch"), + ("agentenv-acme[browser,all]", "torch, playwright>=1.40"), + ]) + def test_an_extra_is_unmet_without_what_it_needs(self, monkeypatch, line, missing): + monkeypatch.setattr(requires_module, "distribution", lambda name: _AcmeMetadata) + + assert unmet([line]) == [(line, f"and agentenv-acme 1.2.0 is installed without {missing}, which its extras " + f"need")] + + @pytest.mark.parametrize("line", ["agentenv-acme[browser]", "agentenv-acme[gpu]", "agentenv-acme[undeclared]"]) + def test_an_extra_whose_needs_are_installed_or_that_needs_nothing_is_met(self, monkeypatch, line): + monkeypatch.setattr(requires_module, "distribution", lambda name: _AcmeMetadata) + monkeypatch.setattr(requires_module, "installed", + {ACME: "1.2.0", "playwright": "1.41", "torch": "2.5"}.get) + + assert unmet([line]) == [] + def test_one_unmet_requirement_names_the_command_that_installs_it(self): assert unmet_problems(["agentenv-missing>=1", "agentenv-acme"]) == ([MISSING], ["agentenv-missing>=1"]) @@ -202,6 +248,14 @@ def test_a_type_only_configuration_registers_is_named_and_never_required(self, t "needs that configuration too"] assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) + def test_a_folder_on_a_filesystem_without_links_is_told_what_to_declare(self, tmp_path, monkeypatch): + monkeypatch.setattr(requires_module.os, "link", _no_links) + root = _bundle(tmp_path / "b", _Acme.type) + + assert _declared(root)[0] == ("this bundle uses agentenv-acme 1.2.0 (tasks/t.json: step 's0'), which bundle.toml " + "doesn't require") + assert sorted(os.listdir(root)) == ["tasks"] + def test_a_bundle_toml_that_appears_meanwhile_is_kept_and_told_what_to_declare(self, tmp_path, monkeypatch): root = _bundle(tmp_path / "b", _Acme.type) resolved = resolve_bundle(parse_bundle(root)) @@ -240,20 +294,13 @@ def create(text): assert path.read_text() in texts assert os.listdir(tmp_path) == ["bundle.toml"] - def test_without_links_it_creates_it_in_place_still_never_over_another(self, tmp_path, monkeypatch): - def no_links(src, dst): - raise PermissionError("links aren't supported here") + def test_a_filesystem_without_links_gets_none(self, tmp_path, monkeypatch): + monkeypatch.setattr(requires_module.os, "link", _no_links) - monkeypatch.setattr(requires_module.os, "link", no_links) - path = tmp_path / "bundle.toml" - - assert _create(path, "first\n") is True - assert _create(path, "second\n") is False - assert path.read_text() == "first\n" - assert os.listdir(tmp_path) == ["bundle.toml"] + assert _create(tmp_path / "bundle.toml", "requires = []\n") is False + assert os.listdir(tmp_path) == [] - @pytest.mark.parametrize("links", [True, False], ids=["staged", "in-place"]) - def test_a_file_it_cant_write_whole_is_never_left_behind(self, tmp_path, monkeypatch, links): + def test_a_file_it_cant_write_whole_is_never_left_behind(self, tmp_path, monkeypatch): real_open = open def full(path, mode="r", **kwargs): @@ -262,8 +309,6 @@ def full(path, mode="r", **kwargs): return file monkeypatch.setattr(requires_module, "open", full, raising=False) - if not links: - monkeypatch.setattr(requires_module.os, "link", lambda src, dst: (_ for _ in ()).throw(PermissionError())) assert _create(tmp_path / "bundle.toml", "requires = []\n") is False assert os.listdir(tmp_path) == [] From 3b5ade93187b56a027b4d942e8608880ba6bf943 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 16:24:04 -0700 Subject: [PATCH 4/6] feat(bundle): name the package a configuration-registered type comes from, and say to upgrade agent-env itself A type only configuration registers is hinted with the installed package its class's module comes from, when one ships it, so whoever runs the bundle knows what to install as well as which configuration it needs. A bundle.toml requiring agent-env itself (agentenv-framework or its protocol) says to upgrade agent-env with the installer that installed it: plugin add refuses to change agent-env, so it's left out of the command and the prompt. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 7 +-- src/agent_env/bundle/requires.py | 76 ++++++++++++++++++++++++-------- tst/unit/bundle/requires_test.py | 59 +++++++++++++++++++++++++ tst/unit/bundle/run_test.py | 2 + 4 files changed, 122 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 340d3ef13..86898fcab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -180,11 +180,12 @@ Out-of-tree code plugs in either through `config.toml`, as a `module:Class` poin - A bundle names the plugin packages it needs in an optional `bundle.toml` at its root (`requires = ["", ...]`, read by `bundle/parse.py`, checked by `bundle/requires.py`). Resolving a bundle refuses one whose requirements this environment doesn't meet (`UnmetRequirements`), naming the `agent-env plugin add` command - that installs them; in a terminal `agent-env run` and `bundle register` offer to run it, then run the command again - in a new process. A run or registration of a writable folder run by its path that has no `bundle.toml` creates one, + that installs them (a requirement on agent-env itself says to upgrade it instead); in a terminal `agent-env run` and + `bundle register` offer to run it, then run the command again in a new process. A run or registration of a writable folder run by its path that has no `bundle.toml` creates one, requiring at least the installed version of each plugin distribution whose type the bundle uses, never over a file that appeared meanwhile. An existing one is never edited: the run says what to add, so `requires = []` keeps a - bundle requiring nothing. A type only configuration registers gets a hint, never a requirement. `bundle init + bundle requiring nothing. A type only configuration registers gets a hint naming the installed package its class + comes from, never a requirement. `bundle init --from-task` writes the same file. - Environment providers have no table: an installed package registers one as an `agent_env.env_providers` entry point named its `type`, which records carry as `env_provider_type`. diff --git a/src/agent_env/bundle/requires.py b/src/agent_env/bundle/requires.py index abbd4a18d..d19c16b65 100644 --- a/src/agent_env/bundle/requires.py +++ b/src/agent_env/bundle/requires.py @@ -9,9 +9,9 @@ import os import shlex import uuid -from collections.abc import Iterable, Sequence +from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass -from importlib.metadata import PackageNotFoundError, distribution +from importlib.metadata import PackageNotFoundError, distribution, packages_distributions from pathlib import Path from typing import TYPE_CHECKING @@ -23,7 +23,7 @@ from agent_env.env import registry as env_registry from agent_env.env.registry import get_env_registry from agent_env.plugins import ARTIFACTS, ENVS, TASK_STEPS, inventory, load_failures -from agent_env.plugins._requirements import installed +from agent_env.plugins._requirements import CORE, installed from agent_env.task_step import registry as task_step_registry from agent_env.task_step.registry import get_task_step_registry @@ -51,10 +51,11 @@ def __init__(self, problems: Iterable[str], requirements: Sequence[str]): @dataclass(frozen=True) class Provider: """What registers a type core doesn't ship: an installed distribution, at its version, or, with neither, the - configuration's impls.""" + configuration's impls, then with the installed package its class's module comes from, when one ships it.""" distribution: str | None version: str | None + shipped_by: str | None = None # " " def requirement(self) -> str | None: """The pip requirement for it, at least the version installed here without a local label, which a bound can't @@ -82,6 +83,7 @@ class Providers: def __init__(self) -> None: self._origins: dict[tuple[str, str], tuple[str, str, str]] | None = None + self._packages: Mapping[str, list[str]] | None = None def of(self, group: str, name: str, cls: type | None) -> Provider | None: """What provides ``name`` of the plugin ``group``, which resolves here to ``cls``: the distribution whose entry @@ -96,7 +98,26 @@ def of(self, group: str, name: str, cls: type | None) -> Provider | None: for contribution in distribution.contributions} distribution, version, value = self._origins.get((group, name), (None, None, None)) registered = value is not None and cls is not None and name not in load_failures().get(group, {}) - return Provider(distribution, version) if registered and _names(value, cls) else Provider(None, None) + if registered and _names(value, cls): + return Provider(distribution, version) + return Provider(None, None, self._shipping(cls)) + + def _shipping(self, cls: type | None) -> str | None: + """The installed package whose files hold ``cls``'s module, at its version; None when no one package does, as + for a module on the path or an editable install.""" + if cls is None: + return None + if self._packages is None: + self._packages = packages_distributions() + module = cls.__module__ + names = list(dict.fromkeys(self._packages.get(module.partition(".")[0], ()))) + if len(names) > 1: # a namespace package: the one whose files hold the module + files = {f"{module.replace('.', '/')}.py", f"{module.replace('.', '/')}/__init__.py"} + names = [name for name in names if files & {str(file) for file in distribution(name).files or ()}] + if len(names) != 1: + return None + version = installed(canonicalize_name(names[0])) + return f"{names[0]} {version}" if version else names[0] def unmet(requires: Iterable[str]) -> list[tuple[str, str]]: @@ -156,18 +177,22 @@ def _missing_for_extras(requirement: Requirement, seen: set[tuple[str, str]]) -> def unmet_problems(requires: Iterable[str]) -> tuple[list[str], list[str]]: - """The problems ``bundle.toml``'s unmet ``requires`` raise, naming the one command that installs them all, and the - unmet requirements.""" + """The problems ``bundle.toml``'s unmet ``requires`` raise, naming the one command that installs every plugin + package among them, and those requirements. One on agent-env itself, which ``plugin add`` doesn't change, says to + upgrade it instead.""" found = unmet(requires) - if not found: - return [], [] - lines = [line for line, _ in found] - command = install_command(lines) - if len(found) == 1: - line, why = found[0] - return [f"{BUNDLE_TOML}: requires {line!r}, {why}; install it with: {command}"], lines - return ([f"{BUNDLE_TOML}: requires {line!r}, {why}" for line, why in found] - + [f"{BUNDLE_TOML}: install what it requires with: {command}"]), lines + core = [(line, why) for line, why in found if canonicalize_name(Requirement(line).name) in CORE] + plugins = [(line, why) for line, why in found if (line, why) not in core] + problems = [f"{BUNDLE_TOML}: requires {line!r}, {why}; upgrade agent-env with the installer that installed it" + for line, why in core] + lines = [line for line, _ in plugins] + if len(plugins) == 1: + (line, why), = plugins + problems.append(f"{BUNDLE_TOML}: requires {line!r}, {why}; install it with: {install_command(lines)}") + elif plugins: + problems += [f"{BUNDLE_TOML}: requires {line!r}, {why}" for line, why in plugins] + problems.append(f"{BUNDLE_TOML}: install what it requires with: {install_command(lines)}") + return problems, lines def install_command(requirements: Sequence[str]) -> str: @@ -214,9 +239,7 @@ def declare(resolved: ResolvedBundle, *, own: bool, dry_run: bool) -> list[str]: declared = {canonicalize_name(Requirement(line).name) for line in resolved.bundle.requires or ()} undeclared = [use for use in found if use.provider.distribution is not None and canonicalize_name(use.provider.distribution) not in declared] - configured = [use for use in found if use.provider.distribution is None] - lines = [f"this bundle uses {_listing(configured)} ({_wheres(configured)}), which this machine's configuration " - "registers; whoever runs it needs that configuration too"] if configured else [] + lines = _configured([use for use in found if use.provider.distribution is None]) if not undeclared: return lines requires = detected(undeclared) @@ -263,6 +286,21 @@ def _create(path: Path, text: str) -> bool: staged.unlink(missing_ok=True) +def _configured(found: Sequence[Use]) -> list[str]: + """A line for the types ``found`` that only this machine's configuration registers: one per installed package + their classes come from, then one for those no package ships.""" + by_package: dict[str | None, list[Use]] = {} + for use in found: + by_package.setdefault(use.provider.shipped_by, []).append(use) + lines = [f"this bundle uses {_listing(uses_of)} ({_wheres(uses_of)}), which this machine's configuration registers, " + f"from {package}; whoever runs it needs that package and its configuration" + for package, uses_of in sorted(by_package.items(), key=lambda item: item[0] or "") if package] + if None in by_package: + lines.append(f"this bundle uses {_listing(by_package[None])} ({_wheres(by_package[None])}), which this " + "machine's configuration registers; whoever runs it needs that configuration too") + return lines + + def _listing(found: Sequence[Use]) -> str: """Each distinct type ``found`` names, once: ``the step type 'a' and the env type 'b'``.""" named = list(dict.fromkeys(f"the {use.what} {use.name!r}" for use in found)) diff --git a/tst/unit/bundle/requires_test.py b/tst/unit/bundle/requires_test.py index 1ed315f83..ac0386be5 100644 --- a/tst/unit/bundle/requires_test.py +++ b/tst/unit/bundle/requires_test.py @@ -7,6 +7,8 @@ import sys import threading import tomllib +from pathlib import PurePosixPath +from types import SimpleNamespace import pytest from click.testing import CliRunner @@ -89,6 +91,7 @@ def acme(monkeypatch): config_path=None, config_error=None, loaded=load)) monkeypatch.setattr(requires_module, "load_failures", dict) monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0"}.get) + monkeypatch.setattr(requires_module, "packages_distributions", dict) def _bundle(root, *types, toml=None): @@ -124,6 +127,21 @@ def test_a_type_configuration_registers_is_provided_by_configuration(self, cls): assert Providers().of(TASK_STEPS, cls.type, cls) == Provider(None, None) assert Provider(None, None).requirement() is None + def test_configuration_names_the_installed_package_its_class_comes_from(self, monkeypatch): + monkeypatch.setattr(requires_module, "packages_distributions", lambda: {"tst": ["agentenv-shipper"]}) + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-shipper": "2.0"}.get) + + assert Providers().of(TASK_STEPS, _Configured.type, _Configured) == Provider(None, None, "agentenv-shipper 2.0") + + def test_of_a_namespace_packages_distributions_the_one_holding_the_module_ships_it(self, monkeypatch): + files = {"shipper-a": ["tst/other.py"], "shipper-b": [f"{__name__.replace('.', '/')}.py"]} + monkeypatch.setattr(requires_module, "packages_distributions", lambda: {"tst": ["shipper-a", "shipper-b"]}) + monkeypatch.setattr(requires_module, "distribution", + lambda name: SimpleNamespace(files=[PurePosixPath(f) for f in files[name]])) + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "shipper-b": "3.1"}.get) + + assert Providers().of(TASK_STEPS, _Configured.type, _Configured).shipped_by == "shipper-b 3.1" + def test_a_plugin_that_failed_to_load_provides_nothing(self, monkeypatch): monkeypatch.setattr(requires_module, "load_failures", lambda: {TASK_STEPS: {_Acme.type: "failed to load"}}) assert Providers().of(TASK_STEPS, _Acme.type, _Acme) == Provider(None, None) @@ -167,6 +185,25 @@ def test_an_extra_whose_needs_are_installed_or_that_needs_nothing_is_met(self, m def test_one_unmet_requirement_names_the_command_that_installs_it(self): assert unmet_problems(["agentenv-missing>=1", "agentenv-acme"]) == ([MISSING], ["agentenv-missing>=1"]) + def test_agent_env_itself_is_upgraded_not_added(self, monkeypatch): + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-framework": "0.9.1375"}.get) + + assert unmet_problems(["agentenv-framework>=0.9.1400", "agentenv-missing>=1"]) == ([ + "bundle.toml: requires 'agentenv-framework>=0.9.1400', and agentenv-framework 0.9.1375 is installed; " + "upgrade agent-env with the installer that installed it", + MISSING, + ], ["agentenv-missing>=1"]) + + def test_a_bundle_needing_a_newer_agent_env_is_refused_without_an_install_to_offer(self, tmp_path, monkeypatch): + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-framework": "0.9.1375"}.get) + root = _bundle(tmp_path / "b", _Acme.type, toml='requires = ["agentenv-framework>=0.9.1400"]\n') + + with pytest.raises(BundleError) as caught: + resolve_bundle(parse_bundle(root)) + + assert not isinstance(caught.value, UnmetRequirements) + assert caught.value.problems[0].endswith("upgrade agent-env with the installer that installed it") + def test_several_name_one_command_that_installs_them_all(self): assert unmet_problems(["agentenv-missing>=1", "agentenv-acme>=2"]) == ([ "bundle.toml: requires 'agentenv-missing>=1', which isn't installed", @@ -256,6 +293,17 @@ def test_a_folder_on_a_filesystem_without_links_is_told_what_to_declare(self, tm "doesn't require") assert sorted(os.listdir(root)) == ["tasks"] + def test_a_type_configuration_registers_from_an_installed_package_names_it(self, tmp_path, monkeypatch): + monkeypatch.setattr(requires_module, "packages_distributions", lambda: {"tst": ["agentenv-shipper"]}) + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-shipper": "2.0"}.get) + root = _bundle(tmp_path / "b", _Configured.type, _Replaced.type) + + assert _declared(root) == [ + "this bundle uses the step type 'configured_requires_test' and the step type 'replaced_requires_test' " + "(tasks/t.json: step 's0', and 1 more), which this machine's configuration registers, from " + "agentenv-shipper 2.0; whoever runs it needs that package and its configuration"] + assert not (root / "bundle.toml").exists() + def test_a_bundle_toml_that_appears_meanwhile_is_kept_and_told_what_to_declare(self, tmp_path, monkeypatch): root = _bundle(tmp_path / "b", _Acme.type) resolved = resolve_bundle(parse_bundle(root)) @@ -491,6 +539,17 @@ def test_an_install_that_exits_non_zero_exits_with_its_code(self, terminal): assert result.exit_code == 2 assert calls["exec"] == [] + def test_a_refusal_for_agent_env_itself_never_asks(self, terminal, monkeypatch): + root, calls = terminal + (root / "bundle.toml").write_text('requires = ["agentenv-framework>=0.9.1400"]\n') + monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-framework": "0.9.1375"}.get) + + result = CliRunner().invoke(cli, ["run", str(root)], input="y\n") + + assert result.exit_code == 1 + assert "upgrade agent-env with the installer that installed it" in result.output + assert "Install" not in result.output and calls["added"] == [] + def test_a_registration_refused_for_it_asks_too(self, terminal): root, calls = terminal diff --git a/tst/unit/bundle/run_test.py b/tst/unit/bundle/run_test.py index ef732f8c7..7f52e04ca 100644 --- a/tst/unit/bundle/run_test.py +++ b/tst/unit/bundle/run_test.py @@ -9,6 +9,7 @@ import pytest from click.testing import CliRunner +import agent_env.bundle.requires as requires_module import agent_env.bundle.run as run_module from agent_env.providers.sandbox_providers import host_images from agent_env.artifact.artifacts.docker_image import DockerImageArtifact @@ -139,6 +140,7 @@ def registries(monkeypatch): steps = {**Config().task_step_registry(), **{cls.type: cls for cls in (_Scored, _Failing, _Cancelled, _NamesAgent, _RecordsModels)}} monkeypatch.setattr(Config, "task_step_registry", lambda self: steps) + monkeypatch.setattr(requires_module, "packages_distributions", dict) # no installed package ships these classes monkeypatch.setattr(_Scored, "seen", []) monkeypatch.setattr(_Scored, "peak", 0) From e832512987a7fc8e3d1f64da408f567b52a6971c Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 18:41:28 -0700 Subject: [PATCH 5/6] fix(bundle): refuse a requirement that can't be printed, and tell no one running an installed bundle what to declare A bundle.toml string can carry control or format characters, which the install prompt and the plugin add command would print as they are, so a shared bundle could make the prompt show one package while installing another. Such a requirement is refused where the file is read. An installed bundle's runner can't edit it, so runs by name, the A2A validator's writes and an installed bundle served by OpenEnv say nothing of what to declare; its requirements are still checked. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/parse.py | 4 ++++ src/agent_env/bundle/run.py | 19 ++++++++++--------- tst/unit/bundle/parse_test.py | 10 ++++++++++ tst/unit/bundle/requires_test.py | 18 ++++++++++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/agent_env/bundle/parse.py b/src/agent_env/bundle/parse.py index faf14feec..a31ba1e1e 100644 --- a/src/agent_env/bundle/parse.py +++ b/src/agent_env/bundle/parse.py @@ -215,6 +215,10 @@ def _requires(self) -> tuple[str, ...] | None: '["agentenv-browser>=1.2"]') return () for line in requires: + if not line.isprintable(): + self._problem(self.bundle_toml, f"requires: {line!r} holds a character that can't be printed, so it " + "can't be shown as written before it's installed") + continue try: requirement = Requirement(line) except InvalidRequirement as e: diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index a384004ff..e8a5e40b2 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -290,7 +290,7 @@ def _write_bundle( say = _progress(on_progress) with namespace_routing(): plan, preflight = _planned(root, (), (), id_root, None, None) - _say_plan(plan, say, own=False, dry_run=False) + _say_plan(plan, say, own=False, dry_run=False, installed=id_root is not None) return _written_with_infra(plan, preflight, say) @@ -331,7 +331,7 @@ def dry_run_bundle( say = _progress(on_progress) with namespace_routing(): plan, preflight = _planned(root, tasks, evals, id_root, sandbox, agent) - _say_plan(plan, say, own=_own(root, id_root), dry_run=True) + _say_plan(plan, say, own=_own(root, id_root), dry_run=True, installed=id_root is not None) materialization = materialize(plan, dry_run=True, on_write=lambda done: say(_written(plan, done))) runs = _to_run(plan) return DryRun(materialization, runs, _skipped(plan, runs, every=not tasks and not evals), preflight.infra, @@ -362,7 +362,7 @@ def _bundle_run(plan: Plan, materialization: Materialization, runs: tuple[TaskRu def _prepare(root: Path | str, tasks: Sequence[str], evals: Sequence[str], sandbox: str | None, id_root: str | None, agent: str | None, played: bool, say: Callable[[str], None], *, own: bool) -> PreparedBundle: plan, preflight = _planned(root, tasks, evals, id_root, sandbox, agent, played=played) - _say_plan(plan, say, own=own, dry_run=False) + _say_plan(plan, say, own=own, dry_run=False, installed=id_root is not None) materialization = _written_with_infra(plan, preflight, say) to_run = tuple((entry, Task.get(entry.id, materialization.version_of("task", entry.id))) for entry in _to_run(plan)) return PreparedBundle(plan, materialization, to_run) @@ -407,10 +407,10 @@ def refuse_store_tasks(plan: Plan) -> None: raise BundleError(problems) -def _say_plan(plan: Plan, say: Callable[[str], None], *, own: bool, dry_run: bool) -> None: - """Say what each agent, env and artifact alias names, what the solver deploys in place of the tasks' agents, and - what plugin packages the bundle uses that its bundle.toml doesn't require, giving a folder of its ``own`` one - (``declare``).""" +def _say_plan(plan: Plan, say: Callable[[str], None], *, own: bool, dry_run: bool, installed: bool) -> None: + """Say what each agent, env and artifact alias names, what the solver deploys in place of the tasks' agents, and, + unless the bundle is ``installed`` (whoever runs it can't edit it), what plugin packages it uses that its + bundle.toml doesn't require, giving a folder of its ``own`` one (``declare``).""" for use in (use for uses in plan.alias_uses.values() for use in uses.values()): say(_aliased(use)) if plan.agent is not None: @@ -418,8 +418,9 @@ def _say_plan(plan: Plan, say: Callable[[str], None], *, own: bool, dry_run: boo solved = sum(entry.id in plan.agent.steps for entry in runs) say(f"--agent {plan.agent.name}: the solver deploys {plan.agent.agent_id}, in {solved} of {len(runs)} " f"task{'s' if len(runs) != 1 else ''}") - for line in declare(plan.bundle, own=own, dry_run=dry_run): - say(line) + if not installed: + for line in declare(plan.bundle, own=own, dry_run=dry_run): + say(line) def _own(root: Path | str, id_root: str | None) -> bool: diff --git a/tst/unit/bundle/parse_test.py b/tst/unit/bundle/parse_test.py index d203d32b7..699c551c9 100644 --- a/tst/unit/bundle/parse_test.py +++ b/tst/unit/bundle/parse_test.py @@ -250,6 +250,16 @@ def test_a_bundle_toml_it_cant_use_is_a_problem(self, tmp_path, text, expected): root = build(tmp_path / "b", {"bundle.toml": text, "tasks/t.json": "[]"}) assert problems(root)[0].startswith(expected), problems(root) + @pytest.mark.parametrize("escape, shown", [("\\u001b[2K\\r", r"\x1b[2K\r"), ("\\u202e", r"\u202e")], + ids=["terminal-escape", "right-to-left-override"]) + def test_a_requirement_that_cant_be_shown_as_written_is_a_problem(self, tmp_path, escape, shown): + text = f'requires = ["evil; os_name != \\"{escape}agentenv-browser\\""]\n' + root = build(tmp_path / "b", {"bundle.toml": text, "tasks/t.json": "[]"}) + + assert problems(root) == ( + f"bundle.toml: requires: 'evil; os_name != \"{shown}agentenv-browser\"' holds a character that can't be " + "printed, so it can't be shown as written before it's installed",) + def test_bundle_toml_that_isnt_utf8_is_a_problem(self, tmp_path): root = build(tmp_path / "b", {"bundle.toml": b"requires = ['\xff']\n", "tasks/t.json": "[]"}) assert problems(root)[0].startswith("bundle.toml: isn't valid TOML: ") diff --git a/tst/unit/bundle/requires_test.py b/tst/unit/bundle/requires_test.py index ac0386be5..83c9de521 100644 --- a/tst/unit/bundle/requires_test.py +++ b/tst/unit/bundle/requires_test.py @@ -22,6 +22,7 @@ Provider, Providers, UnmetRequirements, _create, declare, install_command, unmet, unmet_problems, ) from agent_env.bundle.resolve import resolve_bundle +from agent_env.bundle.run import _write_bundle from agent_env.cli import _install_prompt, cli from agent_env.cli._installers import InstallerError from agent_env.config.runtime import Config @@ -419,6 +420,23 @@ def test_a_registration_writes_it_unless_a_dry_run(self, stores, monkeypatch): assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) assert register_bundle(root).requires == () + def test_an_installed_bundle_is_never_told_what_to_declare_but_is_still_checked(self, stores): + root = _bundle(stores / "b", _Acme.type, _Configured.type) + installed = "@local/agentenv-acme/b" + lines = [] + + run_bundle(root, id_root=installed, on_progress=lines.append) + dry_run_bundle(root, id_root=installed, on_progress=lines.append) + _write_bundle(root, id_root=installed, on_progress=lines.append) + with namespace_routing(): + prepare_bundle(root, id_root=installed, on_progress=lines.append) + + assert not any("bundle.toml" in line or "configuration" in line for line in lines), lines + assert not (root / "bundle.toml").exists() + (root / "bundle.toml").write_text('requires = ["agentenv-missing>=1"]\n') + with pytest.raises(UnmetRequirements): + run_bundle(root, id_root=installed) + def test_the_cli_registration_writes_it_unless_a_dry_run_and_says_so_once(self, stores, monkeypatch): monkeypatch.setattr(register_module, "_git", lambda root, *args: None) root = _bundle(stores / "b", _Acme.type) From 70f92ebe8d5a612a14f1907b036af4b935d075a2 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 28 Oct 2026 21:03:34 -0700 Subject: [PATCH 6/6] refactor(bundle): one detection model, one declaring setting, and the installed-package check beside the plugin one The importer's needs are the same Use records a run detects, with their Provider, and its bundle.toml comes from detected(), so how a requirement is written lives in one place. A Declaring setting (write, would write, hint, quiet), worked out once by declaring_for, replaces the own, dry_run and installed flags and register's write_requires. unmet() and the extras check move to plugins/_requirements.py, beside incompatibility(), which now shares their version comparison. The tests stand in for installed packages through one fake_plugins helper. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_import_report.py | 26 +++--- src/agent_env/bundle/importer.py | 10 +-- src/agent_env/bundle/register.py | 10 +-- src/agent_env/bundle/requires.py | 111 ++++++++----------------- src/agent_env/bundle/run.py | 34 ++++---- src/agent_env/cli/bundle.py | 8 +- src/agent_env/plugins/_requirements.py | 70 ++++++++++++++-- tst/unit/bundle/_support.py | 23 +++++ tst/unit/bundle/importer_test.py | 29 +++---- tst/unit/bundle/requires_test.py | 81 ++++++++++-------- tst/unit/bundle/run_test.py | 5 +- 11 files changed, 221 insertions(+), 186 deletions(-) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index c8b941286..8aa286f3a 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -17,6 +17,8 @@ from agent_env.utils.docker.lock import IN_IMAGE, Locked from agent_env.utils.docker.prune import Pruned +from .requires import Use + # The file an import records what it wrote in, and from where, for publishing the folder back. PROVENANCE = "provenance.json" @@ -50,17 +52,6 @@ class Dropped: why: str -@dataclass(frozen=True) -class Need: - """Something a task uses that core doesn't ship, and what registers it here.""" - - where: str - what: str # step type, env type or artifact type - name: str - by: str # the installed distribution that registers it, or "configuration" for a config file's impls - version: str | None = None # the distribution's, installed here - - @dataclass(frozen=True) class Written: """An entity written into the folder.""" @@ -125,7 +116,7 @@ class BundleImport: pinned: tuple[Pinned, ...] floating: tuple[Floating, ...] dropped: tuple[Dropped, ...] - needs: tuple[Need, ...] + needs: tuple[Use, ...] # what the task uses that core doesn't ship undeclared: tuple[str, ...] # the steps whose type declares no references, so any they make float duplicates: tuple[str, ...] # the eval's entries that named a task it already named downloaded: int # bytes, of what the store said its objects' sizes are @@ -135,7 +126,7 @@ def report(self) -> list[tuple[str, list[str]]]: secret and scoped field was treated, as titled lists of lines; an empty section is left out.""" needed: dict[tuple[str, str, str], list[str]] = {} for need in self.needs: - needed.setdefault((need.by, need.name, need.what), []).append(need.where) + needed.setdefault((_by(need), need.name, need.what), []).append(need.where) return _sections([ ("Still needs the store", [f"{pin.kind.value} {pin.id} v{pin.version}: {pin.where}: {pin.why}" for pin in self.pinned]), @@ -245,9 +236,14 @@ def _listed(items: list[str]) -> str: ] +def _by(need: Use) -> str: + """What registers ``need`` here, as the report names it: the installed distribution, or configuration.""" + return need.provider.distribution or "configuration" + + def _needs_lines(imported: BundleImport) -> list[str]: - plugins = sorted({need.by for need in imported.needs if need.by != "configuration"}) - configured = any(need.by == "configuration" for need in imported.needs) + plugins = sorted({need.provider.distribution for need in imported.needs if need.provider.distribution}) + configured = any(need.provider.distribution is None for need in imported.needs) lines = [f"- The plugins it uses: `{'`, `'.join(plugins)}`."] if plugins else [] if configured: lines.append("- The configuration that registers what *Needs beyond core* says configuration registers.") diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index 0cf62d32f..4b7bd74da 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -29,7 +29,6 @@ Floating, ImportedEval, ImportedTask, - Need, Pinned, Pulled, Written, @@ -51,7 +50,7 @@ segment_problem, ) from agent_env.bundle.plan import STORE_GETTERS, check_bundle -from agent_env.bundle.requires import Provider, Providers +from agent_env.bundle.requires import Providers, Use, detected from agent_env.bundle.resolve import resolve_bundle from agent_env.config import get_config from agent_env.entity.export import ExportContext, Unwritable, checked_path @@ -170,7 +169,7 @@ def __init__(self, dest: Path, include_scopes: Collection[FieldScope] = (), self.pinned: list[Pinned] = [] self.floating: list[Floating] = [] self.dropped: list[Dropped] = [] - self.needs: list[Need] = [] + self.needs: list[Use] = [] self.undeclared: list[str] = [] self.duplicates: list[str] = [] self.problems: list[str] = [] @@ -287,8 +286,7 @@ def write(self, description: str, source: dict) -> BundleImport: if self.problems: raise BundleError(self.problems) shutil.rmtree(scratch) - if requires := sorted({requirement for need in self.needs if need.by != "configuration" - and (requirement := Provider(need.by, need.version).requirement())}): + if requires := detected(self.needs): self.files[BUNDLE_TOML] = _toml.dumps({"requires": requires}) files = sorted([*self.files, PROVENANCE, "README.md"]) imported = BundleImport( @@ -482,7 +480,7 @@ def _need(self, where: str, what: str, group: str, name: str, cls: type | None) """``name``, of the plugin ``group``, is a need when core doesn't ship it, with what provides ``cls``, the class it resolves to here (``Providers.of``).""" if (provider := self.providers.of(group, name, cls)) is not None: - self.needs.append(Need(where, what, name, provider.distribution or "configuration", provider.version)) + self.needs.append(Use(where, what, name, provider)) def _default_agents(self, step: dict, refs: tuple[EntityRef, ...], where: str) -> None: """A step that leaves an agent field unset falls back to the configured default, which no field pins.""" diff --git a/src/agent_env/bundle/register.py b/src/agent_env/bundle/register.py index 62012641a..3f63165b9 100644 --- a/src/agent_env/bundle/register.py +++ b/src/agent_env/bundle/register.py @@ -43,7 +43,7 @@ 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 .requires import declare, writable +from .requires import Declaring, declare, declaring_for from .run import _label from .resolve import BundleImage, Reference, ResolvedBundle, ResolvedEntry, resolve_bundle @@ -75,14 +75,15 @@ def register_bundle( evals: Sequence[str] = (), prefix: str | None = None, dry_run: bool = False, - write_requires: bool | None = None, + declaring: Declaring | None = None, 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. 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. A folder with no - bundle.toml that uses plugin packages is given one when ``write_requires``, by default when it isn't a dry run. + bundle.toml that uses plugin packages is given one, or told what to declare, as ``declaring`` says: by default as a + run of the folder would (``declaring_for``). 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.""" @@ -101,8 +102,7 @@ def register_bundle( problems = list(_unregistrable(registered, shared)) if problems: raise BundleError(problems) - writes = not dry_run if write_requires is None else write_requires - requires = tuple(declare(resolved, own=writable(bundle.root), dry_run=not writes)) + requires = tuple(declare(resolved, declaring or declaring_for(bundle.root, installed=False, dry_run=dry_run))) say = on_progress or (lambda line: None) materialization = materialize( registered, dry_run=dry_run, ledger=ledger, diff --git a/src/agent_env/bundle/requires.py b/src/agent_env/bundle/requires.py index d19c16b65..7a207d691 100644 --- a/src/agent_env/bundle/requires.py +++ b/src/agent_env/bundle/requires.py @@ -11,19 +11,19 @@ import uuid from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass -from importlib.metadata import PackageNotFoundError, distribution, packages_distributions +from enum import Enum +from importlib.metadata import distribution, packages_distributions from pathlib import Path from typing import TYPE_CHECKING -from packaging.requirements import InvalidRequirement, Requirement +from packaging.requirements import Requirement from packaging.utils import canonicalize_name from packaging.version import InvalidVersion, Version from agent_env.artifact.registry import ARTIFACT_REGISTRY, canonical_type, get_artifact_registry from agent_env.env import registry as env_registry from agent_env.env.registry import get_env_registry -from agent_env.plugins import ARTIFACTS, ENVS, TASK_STEPS, inventory, load_failures -from agent_env.plugins._requirements import CORE, installed +from agent_env.plugins import ARTIFACTS, ENVS, TASK_STEPS, _requirements, inventory, load_failures from agent_env.task_step import registry as task_step_registry from agent_env.task_step.registry import get_task_step_registry @@ -48,6 +48,25 @@ def __init__(self, problems: Iterable[str], requirements: Sequence[str]): self.requirements = tuple(requirements) +class Declaring(Enum): + """What a command says, and does, of the plugin packages a bundle uses that its ``bundle.toml`` doesn't require.""" + + WRITE = "write" # give a bundle without a bundle.toml one; tell one that has one what to add + WOULD_WRITE = "would write" # a dry run's: say what WRITE would write + HINT = "hint" # tell it what to declare, writing nothing: a folder that isn't the bundle's own + QUIET = "quiet" # say nothing: an installed bundle, which whoever runs it can't edit + + +def declaring_for(root: Path, *, installed: bool, dry_run: bool) -> Declaring: + """How a command run on the bundle at ``root`` declares its plugins: nothing for an ``installed`` bundle, a hint + for a folder no file can be created in, and otherwise the file, or in a ``dry_run`` what it would hold.""" + if installed: + return Declaring.QUIET + if not os.access(root, os.W_OK | os.X_OK): + return Declaring.HINT + return Declaring.WOULD_WRITE if dry_run else Declaring.WRITE + + @dataclass(frozen=True) class Provider: """What registers a type core doesn't ship: an installed distribution, at its version, or, with neither, the @@ -116,72 +135,16 @@ def _shipping(self, cls: type | None) -> str | None: names = [name for name in names if files & {str(file) for file in distribution(name).files or ()}] if len(names) != 1: return None - version = installed(canonicalize_name(names[0])) + version = _requirements.installed(canonicalize_name(names[0])) return f"{names[0]} {version}" if version else names[0] -def unmet(requires: Iterable[str]) -> list[tuple[str, str]]: - """Each of ``requires`` that isn't met here, with why: its package isn't installed, is at a version it excludes, or - is without what the extras it names need. One whose marker is false here doesn't apply.""" - found = [] - for line in requires: - requirement = Requirement(line) - if requirement.marker is not None and not requirement.marker.evaluate({"extra": ""}): - continue - have = installed(canonicalize_name(requirement.name)) - if have is None: - found.append((line, "which isn't installed")) - elif not _allows(requirement, have): - found.append((line, f"and {requirement.name} {have} is installed")) - elif missing := _missing_for_extras(requirement, set()): - found.append((line, f"and {requirement.name} {have} is installed without {', '.join(missing)}, which its " - f"extras need")) - return found - - -def _allows(requirement: Requirement, version: str) -> bool: - """Whether ``requirement`` admits ``version``; one pip can't compare it admits only when it names no versions.""" - try: - return requirement.specifier.contains(version, prereleases=True) - except InvalidVersion: - return not requirement.specifier - - -def _missing_for_extras(requirement: Requirement, seen: set[tuple[str, str]]) -> list[str]: - """What the extras ``requirement`` names need of its installed package that isn't installed, or not at a version - it allows, following an extra that names another of the package's own.""" - name = canonicalize_name(requirement.name) - missing = [] - for extra in sorted(requirement.extras): - if (name, extra) in seen: - continue - seen.add((name, extra)) - try: - lines = distribution(name).requires or () - except PackageNotFoundError: - return [] - for line in lines: - try: - needed = Requirement(line) - except InvalidRequirement: - continue - if needed.marker is None or not needed.marker.evaluate({"extra": extra}) \ - or needed.marker.evaluate({"extra": ""}): - continue - have = installed(canonicalize_name(needed.name)) - if have is None or not _allows(needed, have): - missing.append(f"{needed.name}{needed.specifier}") - else: - missing.extend(_missing_for_extras(needed, seen)) - return list(dict.fromkeys(missing)) - - def unmet_problems(requires: Iterable[str]) -> tuple[list[str], list[str]]: """The problems ``bundle.toml``'s unmet ``requires`` raise, naming the one command that installs every plugin package among them, and those requirements. One on agent-env itself, which ``plugin add`` doesn't change, says to upgrade it instead.""" - found = unmet(requires) - core = [(line, why) for line, why in found if canonicalize_name(Requirement(line).name) in CORE] + found = _requirements.unmet(requires) + core = [(line, why) for line, why in found if canonicalize_name(Requirement(line).name) in _requirements.CORE] plugins = [(line, why) for line, why in found if (line, why) not in core] problems = [f"{BUNDLE_TOML}: requires {line!r}, {why}; upgrade agent-env with the installer that installed it" for line, why in core] @@ -229,12 +192,13 @@ def detected(found: Iterable[Use]) -> list[str]: return sorted({requirement for use in found if (requirement := use.provider.requirement()) is not None}) -def declare(resolved: ResolvedBundle, *, own: bool, dry_run: bool) -> list[str]: +def declare(resolved: ResolvedBundle, declaring: Declaring) -> list[str]: """What to say of the plugin packages the bundle uses that its ``bundle.toml`` doesn't require, and of each type - only this machine's configuration registers. A bundle with no ``bundle.toml`` in a folder of its ``own`` (run by - path, and writable) is given one listing them, created whole or not at all and never over one that appeared - meanwhile; a ``dry_run`` says what it would write instead. Any other bundle is told what to declare: an existing - ``bundle.toml`` is never edited.""" + only this machine's configuration registers, as ``declaring`` says. Writing, a bundle with no ``bundle.toml`` is + given one listing them, created whole or not at all and never over one that appeared meanwhile. Otherwise it's + told what to declare: an existing ``bundle.toml`` is never edited.""" + if declaring is Declaring.QUIET: + return [] found = uses(resolved) declared = {canonicalize_name(Requirement(line).name) for line in resolved.bundle.requires or ()} undeclared = [use for use in found if use.provider.distribution is not None @@ -244,9 +208,9 @@ def declare(resolved: ResolvedBundle, *, own: bool, dry_run: bool) -> list[str]: return lines requires = detected(undeclared) path = resolved.bundle.root / BUNDLE_TOML - if resolved.bundle.requires is None and own: - if dry_run: - return [f"would write {BUNDLE_TOML}: requires = {json.dumps(requires)}", *lines] + if resolved.bundle.requires is None and declaring is Declaring.WOULD_WRITE: + return [f"would write {BUNDLE_TOML}: requires = {json.dumps(requires)}", *lines] + if resolved.bundle.requires is None and declaring is Declaring.WRITE: if _create(path, _toml.dumps({"requires": requires})): return [f"wrote {BUNDLE_TOML}: requires = {json.dumps(requires)}; commit it so whoever you share the " "bundle with knows what to install", *lines] @@ -261,11 +225,6 @@ def declare(resolved: ResolvedBundle, *, own: bool, dry_run: bool) -> list[str]: return [*hints, *lines] -def writable(root: Path) -> bool: - """Whether a file can be created in the folder ``root``.""" - return os.access(root, os.W_OK | os.X_OK) - - def _create(path: Path, text: str) -> bool: """Write ``text`` to ``path`` whole, unless something is there by then; whether it was written. It's staged beside it under a name of its own, as the umask leaves new files, and linked into place, which fails if ``path`` exists; diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index e8a5e40b2..b3b8e11be 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -39,7 +39,7 @@ from .parse import BundleEntry, BundleError, BundleKind, parse_bundle from .plan import AliasUse, Plan, Write, plan_bundle from .preflight import Preflight, preflight_run -from .requires import declare, writable +from .requires import Declaring, declare, declaring_for from .resolve import BuiltImage, BundleImage, Reference, ReferencedImage, resolve_bundle, without_played_agents logger = logging.getLogger(__name__) @@ -241,7 +241,8 @@ def run_bundle( build_sandbox_provider(sandbox) say = _progress(on_progress) with namespace_routing(): - prepared = _prepare(root, tasks, evals, sandbox, id_root, agent, False, say, own=_own(root, id_root)) + prepared = _prepare(root, tasks, evals, sandbox, id_root, agent, False, say, + declaring_for(Path(root), installed=id_root is not None, dry_run=False)) plan, entries = prepared.plan, [entry for entry, _ in prepared.tasks] with Interrupts() as interrupts: overrides = _overrides(sandbox, user_model, judge_model) @@ -279,7 +280,7 @@ def prepare_bundle( ``run_bundle`` raises before its first task runs, except for a running event loop, which it doesn't need.""" if sandbox: build_sandbox_provider(sandbox) - return _prepare(root, tasks, evals, sandbox, id_root, None, played, _progress(on_progress), own=False) + return _prepare(root, tasks, evals, sandbox, id_root, None, played, _progress(on_progress), _hint(id_root)) def _write_bundle( @@ -290,7 +291,7 @@ def _write_bundle( say = _progress(on_progress) with namespace_routing(): plan, preflight = _planned(root, (), (), id_root, None, None) - _say_plan(plan, say, own=False, dry_run=False, installed=id_root is not None) + _say_plan(plan, say, _hint(id_root)) return _written_with_infra(plan, preflight, say) @@ -331,7 +332,7 @@ def dry_run_bundle( say = _progress(on_progress) with namespace_routing(): plan, preflight = _planned(root, tasks, evals, id_root, sandbox, agent) - _say_plan(plan, say, own=_own(root, id_root), dry_run=True, installed=id_root is not None) + _say_plan(plan, say, declaring_for(Path(root), installed=id_root is not None, dry_run=True)) materialization = materialize(plan, dry_run=True, on_write=lambda done: say(_written(plan, done))) runs = _to_run(plan) return DryRun(materialization, runs, _skipped(plan, runs, every=not tasks and not evals), preflight.infra, @@ -360,9 +361,9 @@ def _bundle_run(plan: Plan, materialization: Materialization, runs: tuple[TaskRu def _prepare(root: Path | str, tasks: Sequence[str], evals: Sequence[str], sandbox: str | None, id_root: str | None, - agent: str | None, played: bool, say: Callable[[str], None], *, own: bool) -> PreparedBundle: + agent: str | None, played: bool, say: Callable[[str], None], declaring: Declaring) -> PreparedBundle: plan, preflight = _planned(root, tasks, evals, id_root, sandbox, agent, played=played) - _say_plan(plan, say, own=own, dry_run=False, installed=id_root is not None) + _say_plan(plan, say, declaring) materialization = _written_with_infra(plan, preflight, say) to_run = tuple((entry, Task.get(entry.id, materialization.version_of("task", entry.id))) for entry in _to_run(plan)) return PreparedBundle(plan, materialization, to_run) @@ -407,10 +408,9 @@ def refuse_store_tasks(plan: Plan) -> None: raise BundleError(problems) -def _say_plan(plan: Plan, say: Callable[[str], None], *, own: bool, dry_run: bool, installed: bool) -> None: - """Say what each agent, env and artifact alias names, what the solver deploys in place of the tasks' agents, and, - unless the bundle is ``installed`` (whoever runs it can't edit it), what plugin packages it uses that its - bundle.toml doesn't require, giving a folder of its ``own`` one (``declare``).""" +def _say_plan(plan: Plan, say: Callable[[str], None], declaring: Declaring) -> None: + """Say what each agent, env and artifact alias names, what the solver deploys in place of the tasks' agents, and + what plugin packages the bundle uses that its bundle.toml doesn't require, as ``declaring`` says (``declare``).""" for use in (use for uses in plan.alias_uses.values() for use in uses.values()): say(_aliased(use)) if plan.agent is not None: @@ -418,15 +418,13 @@ def _say_plan(plan: Plan, say: Callable[[str], None], *, own: bool, dry_run: boo solved = sum(entry.id in plan.agent.steps for entry in runs) say(f"--agent {plan.agent.name}: the solver deploys {plan.agent.agent_id}, in {solved} of {len(runs)} " f"task{'s' if len(runs) != 1 else ''}") - if not installed: - for line in declare(plan.bundle, own=own, dry_run=dry_run): - say(line) + for line in declare(plan.bundle, declaring): + say(line) -def _own(root: Path | str, id_root: str | None) -> bool: - """Whether the bundle at ``root`` is a folder run by its path that a file can be created in, not one an installed - package provides.""" - return id_root is None and writable(Path(root)) +def _hint(id_root: str | None) -> Declaring: + """How a service writing a bundle declares its plugins: nothing for an installed bundle, else a hint.""" + return Declaring.QUIET if id_root is not None else Declaring.HINT def _solver_params(plan: Plan, materialization: Materialization) -> dict[str, dict[str, dict]]: diff --git a/src/agent_env/cli/bundle.py b/src/agent_env/cli/bundle.py index 5285f78c4..5277f11e5 100644 --- a/src/agent_env/cli/bundle.py +++ b/src/agent_env/cli/bundle.py @@ -9,7 +9,7 @@ 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.bundle.requires import UnmetRequirements +from agent_env.bundle.requires import Declaring, UnmetRequirements, declaring_for from agent_env.cli._install_prompt import offer_to_install from agent_env.cli.utils import interruptible from agent_env.entity.scopes import FieldScope @@ -100,8 +100,8 @@ def register(folder: Path, tasks: tuple[str, ...], evals: tuple[str, ...], prefi click.echo("Checking:") with interruptible(): try: - planned = register_bundle(folder, dry_run=True, write_requires=not dry_run, on_progress=click.echo, - **options) + planned = register_bundle(folder, dry_run=True, on_progress=click.echo, **options, + declaring=declaring_for(folder, installed=False, dry_run=dry_run)) except UnmetRequirements as e: if not dry_run: offer_to_install(e) @@ -120,7 +120,7 @@ def register(folder: Path, tasks: tuple[str, ...], evals: tuple[str, ...], prefi click.confirm("Write these to the stores above?", abort=True) click.echo("Writing:") with interruptible(): - registered = register_bundle(folder, on_progress=click.echo, **options) + registered = register_bundle(folder, on_progress=click.echo, **options, declaring=Declaring.QUIET) tasks_written = [done for done in registered.materialization.writes if done.write.kind is BundleKind.TASK] if tasks_written: first = tasks_written[0] diff --git a/src/agent_env/plugins/_requirements.py b/src/agent_env/plugins/_requirements.py index 354393bf4..f7f5b1f91 100644 --- a/src/agent_env/plugins/_requirements.py +++ b/src/agent_env/plugins/_requirements.py @@ -1,4 +1,5 @@ -"""Whether an installed plugin's requirements admit the agent-env installed next to it. +"""Whether an installed plugin's requirements admit the agent-env installed next to it, and which of a list of +requirements this environment doesn't meet. Internal. Reads installed metadata only, so it runs before a plugin is imported, and for ``inventory(load=False)``. @@ -7,11 +8,13 @@ from __future__ import annotations import functools -from importlib.metadata import PackageNotFoundError, version +from collections.abc import Iterable +from importlib.metadata import PackageNotFoundError, distribution, version from typing import Any, Optional from packaging.requirements import InvalidRequirement, Requirement from packaging.utils import canonicalize_name +from packaging.version import InvalidVersion # What a plugin may constrain: agent-env, and the protocol package it pins exactly. CORE = ("agentenv-framework", "agentenv-framework-protocol") @@ -44,12 +47,63 @@ def incompatibility(dist: Any) -> Optional[str]: if name not in CORE or (requirement.marker is not None and not requirement.marker.evaluate({"extra": ""})): continue have = installed(name) + if have is not None and not _allows(requirement, have): + unmet.append(f"{name}{requirement.specifier} (installed: {have})") + return f"needs {'; '.join(unmet)}" if unmet else None + + +def unmet(requires: Iterable[str]) -> list[tuple[str, str]]: + """Each of ``requires`` that isn't met here, with why: its package isn't installed, is at a version it excludes, or + is without what the extras it names need. One whose marker is false here doesn't apply.""" + found = [] + for line in requires: + requirement = Requirement(line) + if requirement.marker is not None and not requirement.marker.evaluate({"extra": ""}): + continue + have = installed(canonicalize_name(requirement.name)) if have is None: + found.append((line, "which isn't installed")) + elif not _allows(requirement, have): + found.append((line, f"and {requirement.name} {have} is installed")) + elif missing := _missing_for_extras(requirement, set()): + found.append((line, f"and {requirement.name} {have} is installed without {', '.join(missing)}, which its " + f"extras need")) + return found + + +def _allows(requirement: Requirement, version: str) -> bool: + """Whether ``requirement`` admits the installed ``version``, prereleases included as pip check counts them; one pip + can't compare it admits only when it names no versions.""" + try: + return requirement.specifier.contains(version, prereleases=True) + except InvalidVersion: + return not requirement.specifier + + +def _missing_for_extras(requirement: Requirement, seen: set[tuple[str, str]]) -> list[str]: + """What the extras ``requirement`` names need of its installed package that isn't installed, or not at a version + it allows, following an extra that names another of the package's own.""" + name = canonicalize_name(requirement.name) + missing = [] + for extra in sorted(requirement.extras): + if (name, extra) in seen: continue + seen.add((name, extra)) try: - met = requirement.specifier.contains(have, prereleases=True) - except Exception: - continue - if not met: - unmet.append(f"{name}{requirement.specifier} (installed: {have})") - return f"needs {'; '.join(unmet)}" if unmet else None + lines = distribution(name).requires or () + except PackageNotFoundError: + return [] + for line in lines: + try: + needed = Requirement(line) + except InvalidRequirement: + continue + if needed.marker is None or not needed.marker.evaluate({"extra": extra}) \ + or needed.marker.evaluate({"extra": ""}): + continue + have = installed(canonicalize_name(needed.name)) + if have is None or not _allows(needed, have): + missing.append(f"{needed.name}{needed.specifier}") + else: + missing.extend(_missing_for_extras(needed, seen)) + return list(dict.fromkeys(missing)) diff --git a/tst/unit/bundle/_support.py b/tst/unit/bundle/_support.py index 8aaf3a6f8..42b1c65b5 100644 --- a/tst/unit/bundle/_support.py +++ b/tst/unit/bundle/_support.py @@ -2,14 +2,17 @@ configured store that fails any test reaching it.""" import shutil +from importlib.metadata import PackageNotFoundError from pathlib import Path from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.bundle import parse_bundle +from agent_env.bundle import requires as requires_module from agent_env.bundle.plan import plan_bundle from agent_env.bundle.resolve import BuiltImage, resolve_bundle from agent_env.config import get_config from agent_env.env.published_images import AGENT_IMAGES +from agent_env.plugins import Inventory, _requirements from agent_env.store.document_store import DocumentStore # The digest the registry fixture pins every tag to. @@ -46,6 +49,26 @@ def listing(folder): for path in Path(folder).rglob("*") if not path.is_dir() or path.is_symlink()) +def fake_plugins(monkeypatch, distributions=(), *, versions=None, packages=None, metadata=None, failures=None): + """This machine's packages, as a bundle's requirements and detection read them: the plugin ``distributions`` the + inventory lists, and the ``failures`` of those that didn't load; each installed package's version, by its + canonical name; the packages each top-level module comes from; and each package's ``metadata`` (``requires``, + ``files``).""" + def read(name): + if name not in (metadata or {}): + raise PackageNotFoundError(name) + return metadata[name] + + monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( + distributions=tuple(distributions), group_errors={}, discovery_errors={}, config_path=None, config_error=None, + loaded=load)) + monkeypatch.setattr(requires_module, "load_failures", lambda: failures or {}) + monkeypatch.setattr(requires_module, "packages_distributions", lambda: packages or {}) + monkeypatch.setattr(requires_module, "distribution", read) + monkeypatch.setattr(_requirements, "installed", (versions or {}).get) + monkeypatch.setattr(_requirements, "distribution", read) + + def plan_of(root): return plan_bundle(resolve_bundle(parse_bundle(root))) diff --git a/tst/unit/bundle/importer_test.py b/tst/unit/bundle/importer_test.py index 59f86db25..f20c4f008 100644 --- a/tst/unit/bundle/importer_test.py +++ b/tst/unit/bundle/importer_test.py @@ -15,11 +15,11 @@ from agent_env.artifact import FileArtifact from agent_env.bundle import BundleError, parse_bundle from agent_env.bundle import importer as importer_module -from agent_env.bundle import requires as requires_module from agent_env.bundle.importer import ImportedEval, import_eval, import_task from agent_env.bundle.installed import InstalledBundle from agent_env.bundle.materialize import materialize from agent_env.bundle.plan import plan_bundle +from agent_env.bundle.requires import Provider from agent_env.bundle.resolve import resolve_bundle from agent_env.cli import cli from agent_env.config import get_config @@ -28,12 +28,12 @@ from agent_env.eval import Eval from agent_env.eval.eval import EvalTask from agent_env.entity.scopes import FieldScope -from agent_env.plugins import TASK_STEPS, Contribution, Distribution, Inventory +from agent_env.plugins import TASK_STEPS, Contribution, Distribution from agent_env.store import Filter from agent_env.store.routing import namespace_routing from agent_env.task import Task from agent_env.task_step.task_step import TaskStep -from tst.unit.bundle._support import context_image +from tst.unit.bundle._support import context_image, fake_plugins STEPS = [ {"id": "env", "type": "deploy_env", "env_id": "crm-env"}, @@ -623,9 +623,7 @@ def test_what_only_a_plugin_or_a_config_file_registers_is_named_as_a_need_by_wha monkeypatch.setattr(Config, "artifact_registry", lambda self: artifacts) acme = Distribution("agentenv-acme", "1.0", ( Contribution(TASK_STEPS, _Undeclared.type, f"{_Undeclared.__module__}:{_Undeclared.__qualname__}", "active"),)) - monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( - distributions=(acme,), group_errors={}, discovery_errors={}, config_path=None, config_error=None, loaded=load)) - monkeypatch.setattr(requires_module, "installed", lambda name: "1.0" if name == "agentenv-acme" else None) + fake_plugins(monkeypatch, [acme], versions={"agentenv-acme": "1.0"}) (store / "acme.json").write_text("{}") _AcmeFile.put("acme-data", description="acme rows", file_path=str(store / "acme.json")) _task("acme", [{"id": "env", "type": "deploy_env", "env_id": "crm-env"}, @@ -634,9 +632,9 @@ def test_what_only_a_plugin_or_a_config_file_registers_is_named_as_a_need_by_wha imported = import_task("acme", store / "acme") - assert sorted((need.by, need.name, need.what, need.where) for need in imported.needs) == [ - ("agentenv-acme", _Undeclared.type, "step type", "tasks/acme.json: step 'p'"), - ("configuration", "acme_file_import_test", "artifact type", "tasks/acme.json: step 'load': artifacts[0].id"), + assert sorted((need.name, need.what, need.where, need.provider) for need in imported.needs) == [ + ("acme_file_import_test", "artifact type", "tasks/acme.json: step 'load': artifacts[0].id", Provider(None, None)), + (_Undeclared.type, "step type", "tasks/acme.json: step 'p'", Provider("agentenv-acme", "1.0")), ] assert imported.report()[0] == ("Needs beyond core", [ "agentenv-acme: undeclared_import_test (step type), at tasks/acme.json: step 'p'", @@ -651,12 +649,10 @@ def test_a_plugins_type_a_config_file_replaces_is_credited_to_configuration(stor monkeypatch.setattr(Config, "task_step_registry", lambda self: steps) replaced = Distribution("agentenv-acme", "1.0", ( Contribution(TASK_STEPS, _Undeclared.type, f"{_AcmeFile.__module__}:{_AcmeFile.__qualname__}", "replaced"),)) - monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( - distributions=(replaced,), group_errors={}, discovery_errors={}, config_path=None, config_error=None, - loaded=load)) + fake_plugins(monkeypatch, [replaced]) _task("acme", [{"id": "p", "type": _Undeclared.type}]) - assert [need.by for need in import_task("acme", store / "acme").needs] == ["configuration"] + assert [need.provider for need in import_task("acme", store / "acme").needs] == [Provider(None, None)] def test_a_skipped_plugins_type_a_config_file_supplies_is_credited_to_configuration_without_loading_it( @@ -669,13 +665,10 @@ def test_a_skipped_plugins_type_a_config_file_supplies_is_credited_to_configurat monkeypatch.syspath_prepend(str(site)) broken = Distribution("agentenv-acme", "1.0", ( Contribution(TASK_STEPS, _Undeclared.type, "acme_broken_import_test:Step", "unloaded"),)) - monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( - distributions=(broken,), group_errors={}, discovery_errors={}, config_path=None, config_error=None, - loaded=load)) - monkeypatch.setattr(requires_module, "load_failures", lambda: {TASK_STEPS: {_Undeclared.type: "failed to load"}}) + fake_plugins(monkeypatch, [broken], failures={TASK_STEPS: {_Undeclared.type: "failed to load"}}) _task("acme", [{"id": "p", "type": _Undeclared.type}]) - assert [need.by for need in import_task("acme", store / "acme").needs] == ["configuration"] + assert [need.provider for need in import_task("acme", store / "acme").needs] == [Provider(None, None)] def test_a_task_that_uses_only_what_core_ships_needs_nothing(store): diff --git a/tst/unit/bundle/requires_test.py b/tst/unit/bundle/requires_test.py index 83c9de521..ec296e9c1 100644 --- a/tst/unit/bundle/requires_test.py +++ b/tst/unit/bundle/requires_test.py @@ -19,17 +19,18 @@ from agent_env.bundle import requires as requires_module from agent_env.bundle.register import register_bundle from agent_env.bundle.requires import ( - Provider, Providers, UnmetRequirements, _create, declare, install_command, unmet, unmet_problems, + Declaring, Provider, Providers, UnmetRequirements, _create, declare, declaring_for, install_command, unmet_problems, ) from agent_env.bundle.resolve import resolve_bundle from agent_env.bundle.run import _write_bundle from agent_env.cli import _install_prompt, cli from agent_env.cli._installers import InstallerError from agent_env.config.runtime import Config -from agent_env.plugins import TASK_STEPS, Contribution, Distribution, Inventory +from agent_env.plugins import TASK_STEPS, Contribution, Distribution +from agent_env.plugins._requirements import unmet from agent_env.store.routing import namespace_routing from agent_env.task_step.task_step import TaskStep -from tst.unit.bundle._support import layout +from tst.unit.bundle._support import fake_plugins, layout ACME = "agentenv-acme" WROTE = ('wrote bundle.toml: requires = ["agentenv-acme>=1.2.0"]; commit it so whoever you share the bundle with knows ' @@ -85,14 +86,16 @@ def acme(monkeypatch): configuration registers and one configuration registers over agentenv-acme's.""" steps = {**Config().task_step_registry(), **{cls.type: cls for cls in (_Acme, _Configured, _Replaced)}} monkeypatch.setattr(Config, "task_step_registry", lambda self: steps) + _plugins(monkeypatch) + + +def _plugins(monkeypatch, *, versions=None, **more): + """agentenv-acme 1.2.0 installed, as the plugin providing ``_Acme`` and the one a config file replaces + ``_Replaced`` of, with ``versions`` of other packages beside it.""" contributions = (Contribution(TASK_STEPS, _Acme.type, _value(_Acme), "active"), Contribution(TASK_STEPS, _Replaced.type, _value(_Acme), "replaced")) - monkeypatch.setattr(requires_module, "inventory", lambda load: Inventory( - distributions=(Distribution(ACME, "1.2.0", contributions),), group_errors={}, discovery_errors={}, - config_path=None, config_error=None, loaded=load)) - monkeypatch.setattr(requires_module, "load_failures", dict) - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0"}.get) - monkeypatch.setattr(requires_module, "packages_distributions", dict) + fake_plugins(monkeypatch, [Distribution(ACME, "1.2.0", contributions)], versions={ACME: "1.2.0", **(versions or {})}, + **more) def _bundle(root, *types, toml=None): @@ -102,8 +105,8 @@ def _bundle(root, *types, toml=None): return layout(root, files) -def _declared(root, *, own=True, dry_run=False): - return declare(resolve_bundle(parse_bundle(root)), own=own, dry_run=dry_run) +def _declared(root, declaring=Declaring.WRITE): + return declare(resolve_bundle(parse_bundle(root)), declaring) class TestProviders: @@ -129,22 +132,19 @@ def test_a_type_configuration_registers_is_provided_by_configuration(self, cls): assert Provider(None, None).requirement() is None def test_configuration_names_the_installed_package_its_class_comes_from(self, monkeypatch): - monkeypatch.setattr(requires_module, "packages_distributions", lambda: {"tst": ["agentenv-shipper"]}) - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-shipper": "2.0"}.get) + _plugins(monkeypatch, versions={"agentenv-shipper": "2.0"}, packages={"tst": ["agentenv-shipper"]}) assert Providers().of(TASK_STEPS, _Configured.type, _Configured) == Provider(None, None, "agentenv-shipper 2.0") def test_of_a_namespace_packages_distributions_the_one_holding_the_module_ships_it(self, monkeypatch): files = {"shipper-a": ["tst/other.py"], "shipper-b": [f"{__name__.replace('.', '/')}.py"]} - monkeypatch.setattr(requires_module, "packages_distributions", lambda: {"tst": ["shipper-a", "shipper-b"]}) - monkeypatch.setattr(requires_module, "distribution", - lambda name: SimpleNamespace(files=[PurePosixPath(f) for f in files[name]])) - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "shipper-b": "3.1"}.get) + _plugins(monkeypatch, versions={"shipper-b": "3.1"}, packages={"tst": ["shipper-a", "shipper-b"]}, + metadata={name: SimpleNamespace(files=[PurePosixPath(f) for f in paths]) for name, paths in files.items()}) assert Providers().of(TASK_STEPS, _Configured.type, _Configured).shipped_by == "shipper-b 3.1" def test_a_plugin_that_failed_to_load_provides_nothing(self, monkeypatch): - monkeypatch.setattr(requires_module, "load_failures", lambda: {TASK_STEPS: {_Acme.type: "failed to load"}}) + _plugins(monkeypatch, failures={TASK_STEPS: {_Acme.type: "failed to load"}}) assert Providers().of(TASK_STEPS, _Acme.type, _Acme) == Provider(None, None) @@ -159,7 +159,7 @@ def test_a_package_not_installed_or_at_a_version_it_excludes_is_unmet(self): ("agentenv-missing", "which isn't installed"), ("agentenv-acme>=2", "and agentenv-acme 1.2.0 is installed")] def test_a_version_pip_cant_compare_meets_only_a_requirement_naming_no_versions(self, monkeypatch): - monkeypatch.setattr(requires_module, "installed", {ACME: "not a version"}.get) + _plugins(monkeypatch, versions={ACME: "not a version"}) assert unmet(["agentenv-acme", "agentenv-acme>=1"]) == [ ("agentenv-acme>=1", "and agentenv-acme not a version is installed")] @@ -170,16 +170,14 @@ def test_a_version_pip_cant_compare_meets_only_a_requirement_naming_no_versions( ("agentenv-acme[browser,all]", "torch, playwright>=1.40"), ]) def test_an_extra_is_unmet_without_what_it_needs(self, monkeypatch, line, missing): - monkeypatch.setattr(requires_module, "distribution", lambda name: _AcmeMetadata) + _plugins(monkeypatch, metadata={ACME: _AcmeMetadata}) assert unmet([line]) == [(line, f"and agentenv-acme 1.2.0 is installed without {missing}, which its extras " f"need")] @pytest.mark.parametrize("line", ["agentenv-acme[browser]", "agentenv-acme[gpu]", "agentenv-acme[undeclared]"]) def test_an_extra_whose_needs_are_installed_or_that_needs_nothing_is_met(self, monkeypatch, line): - monkeypatch.setattr(requires_module, "distribution", lambda name: _AcmeMetadata) - monkeypatch.setattr(requires_module, "installed", - {ACME: "1.2.0", "playwright": "1.41", "torch": "2.5"}.get) + _plugins(monkeypatch, versions={"playwright": "1.41", "torch": "2.5"}, metadata={ACME: _AcmeMetadata}) assert unmet([line]) == [] @@ -187,7 +185,7 @@ def test_one_unmet_requirement_names_the_command_that_installs_it(self): assert unmet_problems(["agentenv-missing>=1", "agentenv-acme"]) == ([MISSING], ["agentenv-missing>=1"]) def test_agent_env_itself_is_upgraded_not_added(self, monkeypatch): - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-framework": "0.9.1375"}.get) + _plugins(monkeypatch, versions={"agentenv-framework": "0.9.1375"}) assert unmet_problems(["agentenv-framework>=0.9.1400", "agentenv-missing>=1"]) == ([ "bundle.toml: requires 'agentenv-framework>=0.9.1400', and agentenv-framework 0.9.1375 is installed; " @@ -196,7 +194,7 @@ def test_agent_env_itself_is_upgraded_not_added(self, monkeypatch): ], ["agentenv-missing>=1"]) def test_a_bundle_needing_a_newer_agent_env_is_refused_without_an_install_to_offer(self, tmp_path, monkeypatch): - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-framework": "0.9.1375"}.get) + _plugins(monkeypatch, versions={"agentenv-framework": "0.9.1375"}) root = _bundle(tmp_path / "b", _Acme.type, toml='requires = ["agentenv-framework>=0.9.1400"]\n') with pytest.raises(BundleError) as caught: @@ -239,10 +237,28 @@ def test_a_bundle_without_one_is_given_a_bundle_toml_listing_the_plugins_it_uses assert parse_bundle(root).requires == ("agentenv-acme>=1.2.0",) assert _declared(root) == [] + @pytest.mark.parametrize("installed, writable, dry_run, declaring", [ + (True, True, False, Declaring.QUIET), + (False, False, False, Declaring.HINT), + (False, False, True, Declaring.HINT), + (False, True, True, Declaring.WOULD_WRITE), + (False, True, False, Declaring.WRITE), + ]) + def test_a_command_declares_as_its_bundle_and_folder_allow(self, tmp_path, monkeypatch, installed, writable, + dry_run, declaring): + monkeypatch.setattr(requires_module.os, "access", lambda path, mode: writable) + assert declaring_for(tmp_path, installed=installed, dry_run=dry_run) is declaring + + def test_quiet_says_and_writes_nothing(self, tmp_path): + root = _bundle(tmp_path / "b", _Acme.type, _Configured.type) + + assert _declared(root, Declaring.QUIET) == [] + assert not (root / "bundle.toml").exists() + def test_a_dry_run_says_what_it_would_write(self, tmp_path): root = _bundle(tmp_path / "b", _Acme.type) - assert _declared(root, dry_run=True) == ['would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]'] + assert _declared(root, Declaring.WOULD_WRITE) == ['would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]'] assert not (root / "bundle.toml").exists() def test_a_bundle_using_only_what_core_ships_is_given_none(self, tmp_path): @@ -257,7 +273,7 @@ def test_a_bundle_using_only_what_core_ships_is_given_none(self, tmp_path): ], ids=["opted-out", "requires-another"]) def test_an_existing_bundle_toml_is_never_edited_but_told_what_to_declare(self, tmp_path, monkeypatch, toml, requires): - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-other": "1"}.get) + _plugins(monkeypatch, versions={"agentenv-other": "1"}) root = _bundle(tmp_path / "b", _Acme.type, toml=toml) assert _declared(root) == [ @@ -269,7 +285,7 @@ def test_an_existing_bundle_toml_is_never_edited_but_told_what_to_declare(self, def test_a_folder_not_its_own_is_told_what_to_declare(self, tmp_path): root = _bundle(tmp_path / "b", _Acme.type, _Acme.type) - assert _declared(root, own=False) == [ + assert _declared(root, Declaring.HINT) == [ "this bundle uses agentenv-acme 1.2.0 (tasks/t.json: step 's0', and 1 more), which bundle.toml doesn't " "require", 'declare them in bundle.toml, so whoever it\'s shared with knows what to install: requires = ' @@ -295,8 +311,7 @@ def test_a_folder_on_a_filesystem_without_links_is_told_what_to_declare(self, tm assert sorted(os.listdir(root)) == ["tasks"] def test_a_type_configuration_registers_from_an_installed_package_names_it(self, tmp_path, monkeypatch): - monkeypatch.setattr(requires_module, "packages_distributions", lambda: {"tst": ["agentenv-shipper"]}) - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-shipper": "2.0"}.get) + _plugins(monkeypatch, versions={"agentenv-shipper": "2.0"}, packages={"tst": ["agentenv-shipper"]}) root = _bundle(tmp_path / "b", _Configured.type, _Replaced.type) assert _declared(root) == [ @@ -310,7 +325,7 @@ def test_a_bundle_toml_that_appears_meanwhile_is_kept_and_told_what_to_declare(s resolved = resolve_bundle(parse_bundle(root)) (root / "bundle.toml").write_text("requires = []\n") - assert declare(resolved, own=True, dry_run=False)[0].startswith("this bundle uses agentenv-acme 1.2.0") + assert declare(resolved, Declaring.WRITE)[0].startswith("this bundle uses agentenv-acme 1.2.0") assert (root / "bundle.toml").read_text() == "requires = []\n" @@ -413,7 +428,7 @@ def test_a_registration_writes_it_unless_a_dry_run(self, stores, monkeypatch): dry = register_bundle(root, dry_run=True).requires assert not (root / "bundle.toml").exists() - registered = register_bundle(root, dry_run=True, write_requires=True).requires + registered = register_bundle(root, dry_run=True, declaring=Declaring.WRITE).requires assert dry == ('would write bundle.toml: requires = ["agentenv-acme>=1.2.0"]',) assert registered == (WROTE,) @@ -560,7 +575,7 @@ def test_an_install_that_exits_non_zero_exits_with_its_code(self, terminal): def test_a_refusal_for_agent_env_itself_never_asks(self, terminal, monkeypatch): root, calls = terminal (root / "bundle.toml").write_text('requires = ["agentenv-framework>=0.9.1400"]\n') - monkeypatch.setattr(requires_module, "installed", {ACME: "1.2.0", "agentenv-framework": "0.9.1375"}.get) + _plugins(monkeypatch, versions={"agentenv-framework": "0.9.1375"}) result = CliRunner().invoke(cli, ["run", str(root)], input="y\n") diff --git a/tst/unit/bundle/run_test.py b/tst/unit/bundle/run_test.py index 7f52e04ca..31d3572dd 100644 --- a/tst/unit/bundle/run_test.py +++ b/tst/unit/bundle/run_test.py @@ -9,7 +9,6 @@ import pytest from click.testing import CliRunner -import agent_env.bundle.requires as requires_module import agent_env.bundle.run as run_module from agent_env.providers.sandbox_providers import host_images from agent_env.artifact.artifacts.docker_image import DockerImageArtifact @@ -28,7 +27,7 @@ from agent_env.task import Task from agent_env.task_step.context import TaskStepContext from agent_env.task_step.task_step import TaskStep -from tst.unit.bundle._support import layout, local_store, plan_of +from tst.unit.bundle._support import fake_plugins, layout, local_store, plan_of ROOT = "@local/~/triage" DRY_RUN = "Dry run: nothing is written or run.\n" @@ -140,7 +139,7 @@ def registries(monkeypatch): steps = {**Config().task_step_registry(), **{cls.type: cls for cls in (_Scored, _Failing, _Cancelled, _NamesAgent, _RecordsModels)}} monkeypatch.setattr(Config, "task_step_registry", lambda self: steps) - monkeypatch.setattr(requires_module, "packages_distributions", dict) # no installed package ships these classes + fake_plugins(monkeypatch) # no installed package ships these classes monkeypatch.setattr(_Scored, "seen", []) monkeypatch.setattr(_Scored, "peak", 0)