Defer broken manifest parse errors to preflight #239
@@ -37,7 +37,7 @@ import shlex
|
||||
import sys
|
||||
from abc import ABC, abstractmethod
|
||||
from contextlib import AbstractContextManager
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, replace
|
||||
from pathlib import Path
|
||||
from typing import Any, Generic, Sequence, TypeVar
|
||||
|
||||
@@ -289,7 +289,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
|
||||
write_launch_metadata,
|
||||
)
|
||||
|
||||
self._validate(spec)
|
||||
spec = self._validate(spec)
|
||||
|
||||
self._preflight()
|
||||
|
||||
@@ -355,16 +355,21 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
|
||||
"""
|
||||
pass
|
||||
|
||||
def _validate(self, spec: BottleSpec) -> None:
|
||||
"""Cross-backend pre-launch checks. Confirms the agent exists
|
||||
and the named skills are present on the host. Subclasses with
|
||||
additional preconditions should override and call
|
||||
`super()._validate(spec)` first."""
|
||||
manifest = spec.manifest
|
||||
manifest.require_agent(spec.agent_name)
|
||||
def _validate(self, spec: BottleSpec) -> BottleSpec:
|
||||
"""Cross-backend pre-launch checks. Parses the selected agent and
|
||||
its bottle (raising ManifestError on invalid content), confirms
|
||||
skills are present on the host, and every git IdentityFile resolves.
|
||||
|
||||
Returns a new BottleSpec whose manifest is fully loaded for the
|
||||
selected agent. Subclasses with additional preconditions should
|
||||
override and call `super()._validate(spec)` first, using the
|
||||
returned spec for further checks."""
|
||||
manifest = spec.manifest.load_for_agent(spec.agent_name)
|
||||
spec = replace(spec, manifest=manifest)
|
||||
agent = manifest.agents[spec.agent_name]
|
||||
self._validate_skills(agent.skills)
|
||||
self._validate_agent_provider_dockerfile(spec)
|
||||
return spec
|
||||
|
||||
def _validate_skills(self, skills: Sequence[str]) -> None:
|
||||
"""Each named skill must be a directory under the host's
|
||||
|
||||
@@ -14,8 +14,9 @@ def cmd_info(argv: list[str]) -> int:
|
||||
parser.add_argument("name", help="agent name defined in bot-bottle.json")
|
||||
args = parser.parse_args(argv)
|
||||
|
||||
manifest = Manifest.resolve(USER_CWD)
|
||||
manifest.require_agent(args.name)
|
||||
names = Manifest.resolve(USER_CWD)
|
||||
names.require_agent(args.name)
|
||||
manifest = names.load_for_agent(args.name)
|
||||
|
||||
agent = manifest.agents[args.name]
|
||||
bottle = manifest.bottle_for(args.name)
|
||||
|
||||
@@ -41,7 +41,7 @@ def cmd_list(argv: list[str]) -> int:
|
||||
|
||||
if args.scope == "available":
|
||||
manifest = Manifest.resolve(USER_CWD)
|
||||
for name in manifest.agents.keys():
|
||||
for name in manifest.all_agent_names:
|
||||
print(name)
|
||||
return 0
|
||||
|
||||
|
||||
+108
-52
@@ -193,9 +193,10 @@ class ManifestBottle:
|
||||
class Manifest:
|
||||
bottles: Mapping[str, ManifestBottle]
|
||||
agents: Mapping[str, ManifestAgent]
|
||||
|
|
||||
# Agents (and agents referencing broken bottles) that failed to load.
|
||||
# Their errors are deferred to preflight rather than raised at load time.
|
||||
broken_agents: Mapping[str, ManifestError] = field(default_factory=dict)
|
||||
# Set by from_md_dirs; empty in from_json_obj (test/programmatic) mode.
|
||||
# Stores the manifest root dirs so load_for_agent can locate files later.
|
||||
home_md: Path | None = field(default=None)
|
||||
cwd_md: Path | None = field(default=None)
|
||||
|
||||
@classmethod
|
||||
def resolve(cls, cwd: str, *, missing_ok: bool = False) -> "Manifest":
|
||||
@@ -252,31 +253,15 @@ class Manifest:
|
||||
home_dir: Path,
|
||||
cwd_dir: Path | None,
|
||||
) -> "Manifest":
|
||||
"""Programmatic entry point. Loads bottles from
|
||||
`<home_dir>/bottles/`, home agents from `<home_dir>/agents/`,
|
||||
and (if `cwd_dir` is passed) cwd agents from
|
||||
`<cwd_dir>/agents/`. Cwd agents override home agents on
|
||||
name collision. A `bottles/` subdir under `cwd_dir` is
|
||||
logged as a warning and ignored.
|
||||
"""Return a names-only Manifest. No file content is read; only
|
||||
filenames are scanned for the agent selector. Full parsing happens
|
||||
later, per-agent, via `load_for_agent`.
|
||||
|
||||
Per-file parse errors are deferred into `broken_agents` rather
|
||||
than raised, so a broken bottle or agent only fails at preflight
|
||||
when that specific agent is selected for launch.
|
||||
A `bottles/` subdir under `cwd_dir` is logged as a warning and
|
||||
ignored — the filesystem layout IS the trust boundary.
|
||||
|
||||
Used by tests to build a Manifest from fixture directories
|
||||
without touching `os.environ`."""
|
||||
bottles_dir = home_dir / "bottles"
|
||||
from .manifest_loader import load_agents_from_dir, load_bottles_from_dir
|
||||
|
||||
bottles, broken_bottle_errors = load_bottles_from_dir(bottles_dir)
|
||||
|
||||
bottle_names = set(bottles.keys())
|
||||
agents_dir = home_dir / "agents"
|
||||
agents, broken_agents = load_agents_from_dir(
|
||||
agents_dir, bottle_names, source="$HOME",
|
||||
broken_bottle_errors=broken_bottle_errors,
|
||||
)
|
||||
|
||||
if cwd_dir is not None:
|
||||
stale_bottles = cwd_dir / "bottles"
|
||||
if stale_bottles.is_dir():
|
||||
@@ -290,15 +275,7 @@ class Manifest:
|
||||
f"live under $HOME/.bot-bottle/bottles/ "
|
||||
f"(PRD 0011). Move them or delete."
|
||||
)
|
||||
cwd_agents_dir = cwd_dir / "agents"
|
||||
cwd_agents, cwd_broken = load_agents_from_dir(
|
||||
cwd_agents_dir, bottle_names, source="$CWD",
|
||||
broken_bottle_errors=broken_bottle_errors,
|
||||
)
|
||||
agents = {**agents, **cwd_agents}
|
||||
broken_agents = {**broken_agents, **cwd_broken}
|
||||
|
||||
return cls(bottles=bottles, agents=agents, broken_agents=broken_agents)
|
||||
return cls(bottles={}, agents={}, home_md=home_dir, cwd_md=cwd_dir)
|
||||
|
didericis marked this conversation as resolved
Outdated
didericis
commented
Also don't load the agents here anymore, just load agent names/defer loading the full agent until preflight Also don't load the agents here anymore, just load agent names/defer loading the full agent until preflight
|
||||
|
||||
@classmethod
|
||||
def from_json_obj(cls, obj: object) -> "Manifest":
|
||||
@@ -325,27 +302,111 @@ class Manifest:
|
||||
|
||||
@property
|
||||
def all_agent_names(self) -> list[str]:
|
||||
"""Sorted list of all agent names, including broken ones.
|
||||
"""Sorted list of all discoverable agent names.
|
||||
|
||||
Broken agents appear in the CLI selector so users can select any
|
||||
agent — the error surfaces only at preflight when launch is
|
||||
attempted."""
|
||||
return sorted(set(self.agents.keys()) | set(self.broken_agents.keys()))
|
||||
In names-only mode (from resolve/from_md_dirs) this scans agent
|
||||
filenames without reading their content. In eager mode (from
|
||||
from_json_obj) it returns the pre-parsed agents' names."""
|
||||
if self.home_md is not None:
|
||||
from .manifest_loader import scan_agent_names
|
||||
home_names = set(scan_agent_names(self.home_md / "agents").keys())
|
||||
cwd_names: set[str] = set()
|
||||
if self.cwd_md is not None:
|
||||
cwd_names = set(scan_agent_names(self.cwd_md / "agents").keys())
|
||||
return sorted(home_names | cwd_names)
|
||||
return sorted(self.agents.keys())
|
||||
|
||||
def load_for_agent(self, agent_name: str) -> "Manifest":
|
||||
"""Parse and return a full Manifest for `agent_name` and its bottle.
|
||||
|
||||
Only the selected agent's file and the bottle files in its extends
|
||||
chain are read. Raises ManifestError if the agent or bottle is
|
||||
invalid. Must be called on a names-only manifest (from resolve).
|
||||
Backends call this at preflight to upgrade the spec's manifest."""
|
||||
if self.home_md is None:
|
||||
# Eager manifest (from_json_obj): already fully loaded; just validate name.
|
||||
if agent_name not in self.agents:
|
||||
available = ", ".join(sorted(self.agents.keys())) or "(none)"
|
||||
raise ManifestError(
|
||||
f"agent '{agent_name}' not defined. Available: {available}"
|
||||
)
|
||||
return self
|
||||
from .manifest_loader import load_bottle_chain_from_dir, scan_agent_names
|
||||
from .manifest_schema import validate_agent_frontmatter_keys
|
||||
from .yaml_subset import YamlSubsetError, parse_frontmatter
|
||||
|
||||
# Locate the agent file; cwd wins over home on name collision.
|
||||
home_agents = scan_agent_names(self.home_md / "agents")
|
||||
cwd_agents: dict[str, Path] = {}
|
||||
if self.cwd_md is not None:
|
||||
cwd_agents = scan_agent_names(self.cwd_md / "agents")
|
||||
merged = {**home_agents, **cwd_agents}
|
||||
|
||||
if agent_name not in merged:
|
||||
available = ", ".join(sorted(merged.keys())) or "(none)"
|
||||
raise ManifestError(
|
||||
f"agent '{agent_name}' not defined. Available: {available}"
|
||||
)
|
||||
|
||||
agent_path = merged[agent_name]
|
||||
try:
|
||||
fm, body = parse_frontmatter(agent_path.read_text())
|
||||
except OSError as e:
|
||||
raise ManifestError(f"could not read {agent_path}: {e}") from e
|
||||
except YamlSubsetError as e:
|
||||
raise ManifestError(f"{agent_path}: {e}") from e
|
||||
|
||||
validate_agent_frontmatter_keys(agent_path, fm.keys())
|
||||
|
||||
bottle_name = fm.get("bottle")
|
||||
if not isinstance(bottle_name, str) or not bottle_name:
|
||||
raise ManifestError(
|
||||
f"agent '{agent_name}' must declare a 'bottle' field "
|
||||
f"naming a defined bottle"
|
||||
)
|
||||
|
||||
# Load the bottle chain (may raise ManifestError).
|
||||
bottles_dir = self.home_md / "bottles"
|
||||
bottle = load_bottle_chain_from_dir(bottle_name, bottles_dir)
|
||||
|
||||
# Build and validate the full ManifestAgent.
|
||||
agent_dict: dict[str, object] = {
|
||||
"bottle": bottle_name,
|
||||
"skills": fm.get("skills", []),
|
||||
"prompt": body.strip(),
|
||||
}
|
||||
if "git-gate" in fm:
|
||||
agent_dict["git-gate"] = fm["git-gate"]
|
||||
agent = ManifestAgent.from_dict(agent_name, agent_dict, {bottle_name})
|
||||
|
||||
return Manifest(
|
||||
bottles={bottle_name: bottle},
|
||||
agents={agent_name: agent},
|
||||
home_md=self.home_md,
|
||||
cwd_md=self.cwd_md,
|
||||
)
|
||||
|
||||
def has_agent(self, name: str) -> bool:
|
||||
return name in self.agents
|
||||
|
||||
def require_agent(self, name: str) -> None:
|
||||
if name in self.broken_agents:
|
||||
raise self.broken_agents[name]
|
||||
"""Check that `name` is a discoverable agent. In names-only mode
|
||||
this checks whether the .md file exists; in eager mode it checks
|
||||
the pre-parsed agents dict. Does NOT parse file content."""
|
||||
if self.has_agent(name):
|
||||
return
|
||||
available = ", ".join(self.agents.keys())
|
||||
if available:
|
||||
msg = f"agent '{name}' not defined in bot-bottle.json. Available: {available}"
|
||||
raise ManifestError(msg)
|
||||
if self.home_md is not None:
|
||||
# Names-only mode: check file existence without parsing.
|
||||
home_path = self.home_md / "agents" / f"{name}.md"
|
||||
cwd_path = (
|
||||
self.cwd_md / "agents" / f"{name}.md"
|
||||
if self.cwd_md else None
|
||||
)
|
||||
if home_path.is_file() or (cwd_path and cwd_path.is_file()):
|
||||
return
|
||||
available = ", ".join(self.all_agent_names) or "(none)"
|
||||
raise ManifestError(
|
||||
f"agent '{name}' not defined in bot-bottle.json (manifest is empty)."
|
||||
f"agent '{name}' not defined. Available: {available}"
|
||||
)
|
||||
|
||||
def has_bottle(self, name: str) -> bool:
|
||||
@@ -379,16 +440,11 @@ class Manifest:
|
||||
|
||||
def bottle_for(self, agent_name: str) -> ManifestBottle:
|
||||
"""Resolve the Bottle the named agent references, with the
|
||||
agent's git.user overlaid on top. The validator guarantees both
|
||||
lookups succeed for a manifest built via from_json_obj.
|
||||
agent's git.user overlaid on top.
|
||||
|
||||
The overlay lives here, the single point both backends call to
|
||||
resolve an agent's bottle, so the docker / smolmachines git
|
||||
provisioners pick up the merged identity unchanged.
|
||||
|
||||
Raises the stored ManifestError for agents that failed to load."""
|
||||
if agent_name in self.broken_agents:
|
||||
raise self.broken_agents[agent_name]
|
||||
provisioners pick up the merged identity unchanged."""
|
||||
bottle = self.bottles[self.agents[agent_name].bottle]
|
||||
merged = self._effective_git_user(agent_name)
|
||||
if merged == bottle.git_user:
|
||||
|
||||
@@ -4,8 +4,6 @@ from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from .manifest_util import ManifestError
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .manifest import ManifestBottle
|
||||
from .manifest_egress import ManifestEgressConfig
|
||||
@@ -24,23 +22,6 @@ def resolve_bottles(raws: dict[str, dict[str, object]]) -> dict[str, ManifestBot
|
||||
return cache
|
||||
|
||||
|
||||
def resolve_bottles_partial(
|
||||
raws: dict[str, dict[str, object]],
|
||||
) -> tuple[dict[str, ManifestBottle], dict[str, ManifestError]]:
|
||||
"""Apply `extends:` chains and return `(good, broken)`.
|
||||
|
||||
Bottles that fail validation (schema errors, bad extends, cycles) are
|
||||
collected in `broken` rather than raising, so unrelated bottles remain
|
||||
usable. Errors for parent bottles propagate to all children that extend
|
||||
them."""
|
||||
cache: dict[str, ManifestBottle] = {}
|
||||
broken: dict[str, ManifestError] = {}
|
||||
for name in raws:
|
||||
if name not in cache and name not in broken:
|
||||
_resolve_one_bottle_partial(name, raws, cache, broken, ())
|
||||
return cache, broken
|
||||
|
||||
|
||||
def _resolve_one_bottle(
|
||||
name: str,
|
||||
raws: dict[str, dict[str, object]],
|
||||
@@ -48,7 +29,7 @@ def _resolve_one_bottle(
|
||||
repos_cache: dict[str, dict[str, object]],
|
||||
seen: tuple[str, ...],
|
||||
) -> ManifestBottle:
|
||||
from .manifest import ManifestBottle
|
||||
from .manifest import ManifestBottle, ManifestError
|
||||
|
||||
if name in cache:
|
||||
return cache[name]
|
||||
@@ -229,67 +210,3 @@ def _merge_egress(
|
||||
routes = parent.routes + child.routes
|
||||
log = child.Log if "log" in child_egress_raw else parent.Log
|
||||
return ManifestEgressConfig(routes=routes, Log=log)
|
||||
|
||||
|
||||
def _resolve_one_bottle_partial(
|
||||
name: str,
|
||||
raws: dict[str, dict[str, object]],
|
||||
cache: dict[str, ManifestBottle],
|
||||
broken: dict[str, ManifestError],
|
||||
seen: tuple[str, ...],
|
||||
) -> None:
|
||||
"""Error-tolerant variant: on failure, adds to `broken` instead of raising."""
|
||||
from .manifest import ManifestBottle
|
||||
|
||||
if name in cache or name in broken:
|
||||
return
|
||||
if name in seen:
|
||||
chain = " -> ".join(seen + (name,))
|
||||
broken[name] = ManifestError(
|
||||
f"bottle '{name}' is in an extends cycle: {chain}"
|
||||
)
|
||||
return
|
||||
|
||||
raw = raws[name]
|
||||
parent_name_raw = raw.get("extends")
|
||||
child_raw = {k: v for k, v in raw.items() if k != "extends"}
|
||||
|
||||
try:
|
||||
if parent_name_raw is None:
|
||||
cache[name] = ManifestBottle.from_dict(name, child_raw)
|
||||
return
|
||||
|
||||
if not isinstance(parent_name_raw, str):
|
||||
broken[name] = ManifestError(
|
||||
f"bottle '{name}' extends must be a string "
|
||||
f"(was {type(parent_name_raw).__name__})"
|
||||
)
|
||||
return
|
||||
|
||||
parent_name: str = parent_name_raw
|
||||
if parent_name == name:
|
||||
broken[name] = ManifestError(
|
||||
f"bottle '{name}' extends itself; remove the self-reference"
|
||||
)
|
||||
return
|
||||
|
||||
if parent_name not in raws:
|
||||
avail = ", ".join(sorted(raws.keys())) or "(none)"
|
||||
broken[name] = ManifestError(
|
||||
f"bottle '{name}' extends '{parent_name}' which is not "
|
||||
f"defined. Available bottles: {avail}"
|
||||
)
|
||||
return
|
||||
|
||||
_resolve_one_bottle_partial(parent_name, raws, cache, broken, seen + (name,))
|
||||
if parent_name in broken:
|
||||
broken[name] = ManifestError(
|
||||
f"bottle '{name}' extends '{parent_name}' which failed to load: "
|
||||
f"{broken[parent_name]}"
|
||||
)
|
||||
return
|
||||
|
||||
parent = cache[parent_name]
|
||||
cache[name] = _merge_bottles(parent, child_raw, name)
|
||||
except ManifestError as exc:
|
||||
broken[name] = exc
|
||||
|
||||
@@ -8,14 +8,13 @@ from typing import TYPE_CHECKING
|
||||
from .log import warn
|
||||
from .manifest_schema import (
|
||||
entity_name_from_path,
|
||||
validate_agent_frontmatter_keys,
|
||||
validate_bottle_frontmatter_keys,
|
||||
)
|
||||
from .manifest_util import ManifestError
|
||||
from .yaml_subset import YamlSubsetError, parse_frontmatter
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .manifest import ManifestAgent, ManifestBottle
|
||||
from .manifest import ManifestBottle
|
||||
|
||||
|
||||
def check_stale_json(dir_path: Path, md_dir: Path, label: str) -> None:
|
||||
@@ -33,73 +32,13 @@ def check_stale_json(dir_path: Path, md_dir: Path, label: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
def load_bottles_from_dir(
|
||||
bottles_dir: Path,
|
||||
) -> tuple[dict[str, ManifestBottle], dict[str, ManifestError]]:
|
||||
"""Walk `<bottles_dir>/*.md`, parse each as a bottle, and return
|
||||
`({name: Bottle}, {name: error})`. Missing dir returns empty dicts.
|
||||
def scan_agent_names(agents_dir: Path) -> dict[str, Path]:
|
||||
"""Scan `<agents_dir>/*.md` for valid filenames and return `{name: path}`.
|
||||
|
||||
Per-file errors are collected in the second dict rather than raised,
|
||||
so an invalid bottle file does not block unrelated bottles or agents."""
|
||||
from .manifest_extends import resolve_bottles_partial
|
||||
|
||||
raws: dict[str, dict[str, object]] = {}
|
||||
broken: dict[str, ManifestError] = {}
|
||||
if not bottles_dir.is_dir():
|
||||
return {}, {}
|
||||
for path in sorted(bottles_dir.glob("*.md")):
|
||||
name = entity_name_from_path(path)
|
||||
if name is None:
|
||||
warn(
|
||||
f"skipping {path}: filename must match "
|
||||
f"[a-z][a-z0-9-]*.md (got {path.name!r})"
|
||||
)
|
||||
continue
|
||||
try:
|
||||
fm, _body = parse_frontmatter(path.read_text())
|
||||
except OSError as e:
|
||||
broken[name] = ManifestError(f"could not read {path}: {e}")
|
||||
continue
|
||||
except YamlSubsetError as e:
|
||||
broken[name] = ManifestError(f"{path}: {e}")
|
||||
continue
|
||||
try:
|
||||
validate_bottle_frontmatter_keys(path, fm.keys())
|
||||
except ManifestError as e:
|
||||
broken[name] = e
|
||||
continue
|
||||
raws[name] = fm
|
||||
|
||||
good, resolve_broken = resolve_bottles_partial(raws)
|
||||
broken.update(resolve_broken)
|
||||
return good, broken
|
||||
|
||||
|
||||
def load_agents_from_dir(
|
||||
agents_dir: Path,
|
||||
bottle_names: set[str],
|
||||
*,
|
||||
source: str, # noqa: F841 — unused, but required by interface
|
||||
broken_bottle_errors: dict[str, ManifestError] | None = None,
|
||||
) -> tuple[dict[str, ManifestAgent], dict[str, ManifestError]]:
|
||||
"""Walk `<agents_dir>/*.md`, parse each as an agent, and return
|
||||
`({name: Agent}, {name: error})`. The Markdown body becomes the
|
||||
agent's prompt. Missing dir returns empty dicts.
|
||||
|
||||
Per-file errors are collected in the second dict rather than raised.
|
||||
Agents referencing a broken bottle are also moved to the error dict
|
||||
so their error surfaces at preflight rather than manifest load time."""
|
||||
from .manifest import ManifestAgent
|
||||
|
||||
broken_bottles = broken_bottle_errors or {}
|
||||
# Agents may reference bottles that failed to resolve; accept those names
|
||||
# during structural parsing so we can detect the broken-bottle case below.
|
||||
all_known_bottles = bottle_names | set(broken_bottles.keys())
|
||||
|
||||
out: dict[str, ManifestAgent] = {}
|
||||
broken: dict[str, ManifestError] = {}
|
||||
No file content is read. Invalid filenames are skipped with a warning."""
|
||||
result: dict[str, Path] = {}
|
||||
if not agents_dir.is_dir():
|
||||
return out, broken
|
||||
return result
|
||||
for path in sorted(agents_dir.glob("*.md")):
|
||||
name = entity_name_from_path(path)
|
||||
if name is None:
|
||||
@@ -108,36 +47,45 @@ def load_agents_from_dir(
|
||||
f"[a-z][a-z0-9-]*.md (got {path.name!r})"
|
||||
)
|
||||
continue
|
||||
try:
|
||||
fm, body = parse_frontmatter(path.read_text())
|
||||
except OSError as e:
|
||||
broken[name] = ManifestError(f"could not read {path}: {e}")
|
||||
continue
|
||||
except YamlSubsetError as e:
|
||||
broken[name] = ManifestError(f"{path}: {e}")
|
||||
continue
|
||||
try:
|
||||
validate_agent_frontmatter_keys(path, fm.keys())
|
||||
agent_dict: dict[str, object] = {
|
||||
"bottle": fm.get("bottle"),
|
||||
"skills": fm.get("skills", []),
|
||||
"prompt": body.strip(),
|
||||
}
|
||||
if "git-gate" in fm:
|
||||
agent_dict["git-gate"] = fm["git-gate"]
|
||||
agent = ManifestAgent.from_dict(name, agent_dict, all_known_bottles)
|
||||
except ManifestError as e:
|
||||
broken[name] = e
|
||||
continue
|
||||
result[name] = path
|
||||
return result
|
||||
|
||||
# Agent parsed fine but its bottle may have failed to resolve.
|
||||
bottle_ref = agent.bottle
|
||||
if bottle_ref in broken_bottles:
|
||||
broken[name] = ManifestError(
|
||||
f"agent '{name}' references bottle '{bottle_ref}' which "
|
||||
f"failed to load: {broken_bottles[bottle_ref]}"
|
||||
|
||||
def load_bottle_chain_from_dir(
|
||||
bottle_name: str, bottles_dir: Path
|
||||
) -> ManifestBottle:
|
||||
"""Load `bottle_name` and its full `extends:` chain from `bottles_dir`,
|
||||
returning the resolved ManifestBottle.
|
||||
|
||||
Only the files in the extends chain are read — unrelated bottle files
|
||||
are never touched. Raises ManifestError on parse or validation failure."""
|
||||
from .manifest_extends import resolve_bottles
|
||||
|
||||
raws: dict[str, dict[str, object]] = {}
|
||||
to_load = [bottle_name]
|
||||
while to_load:
|
||||
name = to_load.pop()
|
||||
if name in raws:
|
||||
continue
|
||||
path = bottles_dir / f"{name}.md"
|
||||
if not path.is_file():
|
||||
avail = ", ".join(
|
||||
p.stem for p in sorted(bottles_dir.glob("*.md")) if p.is_file()
|
||||
) or "(none)"
|
||||
raise ManifestError(
|
||||
f"bottle '{name}' not found at {path}. "
|
||||
f"Available: {avail}"
|
||||
)
|
||||
continue
|
||||
try:
|
||||
fm, _body = parse_frontmatter(path.read_text())
|
||||
except OSError as e:
|
||||
raise ManifestError(f"could not read {path}: {e}") from e
|
||||
except YamlSubsetError as e:
|
||||
raise ManifestError(f"{path}: {e}") from e
|
||||
validate_bottle_frontmatter_keys(path, fm.keys())
|
||||
raws[name] = dict(fm)
|
||||
parent = fm.get("extends")
|
||||
if isinstance(parent, str):
|
||||
to_load.append(parent)
|
||||
|
||||
out[name] = agent
|
||||
return out, broken
|
||||
return resolve_bottles(raws)[bottle_name]
|
||||
|
||||
@@ -217,7 +217,7 @@ class TestAgentGitUserMdLoader(unittest.TestCase):
|
||||
def test_md_agent_git_user_overlays_bottle(self):
|
||||
self._write("bottles/dev.md", _BOTTLE_DEV)
|
||||
self._write("agents/impl.md", _AGENT_WITH_GIT)
|
||||
m = Manifest.resolve(str(self.home))
|
||||
m = Manifest.resolve(str(self.home)).load_for_agent("impl")
|
||||
u = m.bottle_for("impl").git_user
|
||||
self.assertEqual("agent-name", u.name)
|
||||
self.assertEqual("bottle@example.com", u.email)
|
||||
@@ -226,16 +226,17 @@ class TestAgentGitUserMdLoader(unittest.TestCase):
|
||||
m.git_identity_summary("impl"),
|
||||
)
|
||||
|
||||
def test_md_agent_repos_deferred(self):
|
||||
"""git-gate.repos on an agent is an error, but deferred into
|
||||
broken_agents rather than raised at resolve time, so other agents
|
||||
remain accessible."""
|
||||
def test_md_agent_repos_fails_at_preflight(self):
|
||||
"""git-gate.repos on an agent is an error; resolve() still succeeds
|
||||
so other agents remain accessible, but load_for_agent raises."""
|
||||
self._write("bottles/dev.md", _BOTTLE_DEV)
|
||||
self._write("agents/impl.md", _AGENT_WITH_REPOS)
|
||||
m = Manifest.resolve(str(self.home))
|
||||
self.assertNotIn("impl", m.agents)
|
||||
self.assertIn("impl", m.broken_agents)
|
||||
msg = str(m.broken_agents["impl"])
|
||||
from bot_bottle.manifest import ManifestError
|
||||
names = Manifest.resolve(str(self.home))
|
||||
self.assertIn("impl", names.all_agent_names)
|
||||
with self.assertRaises(ManifestError) as ctx:
|
||||
names.load_for_agent("impl")
|
||||
msg = str(ctx.exception)
|
||||
self.assertIn("git-gate.repos", msg)
|
||||
self.assertIn("bottle-only", msg)
|
||||
|
||||
|
||||
@@ -77,12 +77,12 @@ class _ResolveCase(unittest.TestCase):
|
||||
|
||||
class TestBottleFileParses(_ResolveCase):
|
||||
"""SC #1: a bottle file under $HOME/.bot-bottle/bottles/
|
||||
parses into the expected Bottle shape."""
|
||||
parses into the expected Bottle shape via load_for_agent."""
|
||||
|
||||
def test_loads(self):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL)
|
||||
m = self.resolve()
|
||||
m = self.resolve().load_for_agent("implementer")
|
||||
self.assertIn("dev", m.bottles)
|
||||
routes = m.bottles["dev"].egress.routes
|
||||
self.assertEqual(2, len(routes))
|
||||
@@ -94,13 +94,13 @@ class TestBottleFileParses(_ResolveCase):
|
||||
|
||||
class TestAgentFileParses(_ResolveCase):
|
||||
"""SC #2: an agent file under $HOME/.bot-bottle/agents/
|
||||
parses, the body becomes the prompt, the frontmatter fields
|
||||
map to Agent fields."""
|
||||
parses via load_for_agent; the body becomes the prompt, the
|
||||
frontmatter fields map to Agent fields."""
|
||||
|
||||
def test_loads(self):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL)
|
||||
m = self.resolve()
|
||||
m = self.resolve().load_for_agent("implementer")
|
||||
a = m.agents["implementer"]
|
||||
self.assertEqual("dev", a.bottle)
|
||||
self.assertEqual(("init-prd",), a.skills)
|
||||
@@ -128,7 +128,7 @@ class TestCwdAgentOverridesHome(_ResolveCase):
|
||||
CWD-OVERRIDE-PROMPT
|
||||
""",
|
||||
)
|
||||
m = self.resolve()
|
||||
m = self.resolve().load_for_agent("implementer")
|
||||
self.assertIn("CWD-OVERRIDE-PROMPT", m.agents["implementer"].prompt)
|
||||
# Home bottle still present
|
||||
self.assertEqual(2, len(m.bottles["dev"].egress.routes))
|
||||
@@ -155,7 +155,7 @@ class TestCwdBottlesIgnored(_ResolveCase):
|
||||
---
|
||||
""",
|
||||
)
|
||||
m = self.resolve()
|
||||
m = self.resolve().load_for_agent("implementer")
|
||||
# Home value wins because cwd bottles are ignored entirely.
|
||||
self.assertEqual(
|
||||
"api.anthropic.com",
|
||||
@@ -215,7 +215,7 @@ class TestAgentFileDoublesAsClaudeCodeSubagent(_ResolveCase):
|
||||
Agent prompt body.
|
||||
""",
|
||||
)
|
||||
m = self.resolve()
|
||||
m = self.resolve().load_for_agent("implementer")
|
||||
self.assertEqual("dev", m.agents["implementer"].bottle)
|
||||
self.assertEqual(("init-prd",), m.agents["implementer"].skills)
|
||||
|
||||
@@ -228,7 +228,7 @@ class TestManifestEntryPointParity(_ResolveCase):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL)
|
||||
|
||||
md_manifest = self.resolve()
|
||||
md_manifest = self.resolve().load_for_agent("implementer")
|
||||
json_manifest = Manifest.from_json_obj({
|
||||
"bottles": {
|
||||
"dev": {
|
||||
@@ -294,35 +294,12 @@ class TestManifestEntryPointParity(_ResolveCase):
|
||||
self.assertEqual("dev", manifest.agents["implementer"].bottle)
|
||||
|
||||
|
||||
class TestUnknownAgentKeyDefersToBroken(_ResolveCase):
|
||||
"""A typo'd / unknown frontmatter key on an agent file is deferred
|
||||
into broken_agents rather than crashing the whole manifest load.
|
||||
The error surfaces when that specific agent is selected for launch."""
|
||||
class TestBrokenAgentOnlyFailsAtPreflight(_ResolveCase):
|
||||
"""A typo'd / unknown frontmatter key on an agent file does NOT crash
|
||||
resolve(). The agent appears in all_agent_names for the selector.
|
||||
The error surfaces only when load_for_agent is called for that agent."""
|
||||
|
||||
def test_broken_agent_deferred(self):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(
|
||||
self.home_cb / "agents" / "bad.md",
|
||||
"""
|
||||
---
|
||||
bottle: dev
|
||||
skillz: [init-prd]
|
||||
---
|
||||
|
||||
...
|
||||
""",
|
||||
)
|
||||
_write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL)
|
||||
m = self.resolve()
|
||||
# The broken agent is NOT in the valid agents dict…
|
||||
self.assertNotIn("bad", m.agents)
|
||||
# …but it IS captured in broken_agents.
|
||||
self.assertIn("bad", m.broken_agents)
|
||||
self.assertIsInstance(m.broken_agents["bad"], ManifestError)
|
||||
# Unrelated agent still loads fine.
|
||||
self.assertIn("implementer", m.agents)
|
||||
|
||||
def test_broken_agent_appears_in_all_agent_names(self):
|
||||
def test_resolve_succeeds_despite_broken_agent(self):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(
|
||||
self.home_cb / "agents" / "bad.md",
|
||||
@@ -335,10 +312,11 @@ class TestUnknownAgentKeyDefersToBroken(_ResolveCase):
|
||||
)
|
||||
_write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL)
|
||||
m = self.resolve()
|
||||
# Resolve itself does not raise; broken agent appears in the name list.
|
||||
self.assertIn("bad", m.all_agent_names)
|
||||
self.assertIn("implementer", m.all_agent_names)
|
||||
|
||||
def test_broken_agent_raises_at_preflight(self):
|
||||
def test_load_for_agent_raises_for_broken_agent(self):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(
|
||||
self.home_cb / "agents" / "bad.md",
|
||||
@@ -351,17 +329,11 @@ class TestUnknownAgentKeyDefersToBroken(_ResolveCase):
|
||||
)
|
||||
m = self.resolve()
|
||||
with self.assertRaises(ManifestError):
|
||||
m.require_agent("bad")
|
||||
with self.assertRaises(ManifestError):
|
||||
m.bottle_for("bad")
|
||||
m.load_for_agent("bad")
|
||||
|
||||
|
||||
class TestUnknownBottleKeyDefersToBroken(_ResolveCase):
|
||||
"""A typo'd / unknown frontmatter key on a bottle file is deferred
|
||||
into broken_agents for agents referencing that bottle, rather than
|
||||
crashing the whole manifest load."""
|
||||
|
||||
def test_broken_bottle_defers_agent(self):
|
||||
def test_broken_bottle_only_fails_at_preflight(self):
|
||||
"""A broken bottle does not crash resolve; only load_for_agent for
|
||||
an agent that references it raises. Unrelated agents still work."""
|
||||
_write(
|
||||
self.home_cb / "bottles" / "bad.md",
|
||||
"""
|
||||
@@ -382,13 +354,15 @@ class TestUnknownBottleKeyDefersToBroken(_ResolveCase):
|
||||
""",
|
||||
)
|
||||
m = self.resolve()
|
||||
# Good bottle and agent still load.
|
||||
self.assertIn("dev", m.bottles)
|
||||
self.assertIn("implementer", m.agents)
|
||||
# Broken bottle's agent is deferred.
|
||||
self.assertNotIn("bad", m.bottles)
|
||||
self.assertNotIn("broken-agent", m.agents)
|
||||
self.assertIn("broken-agent", m.broken_agents)
|
||||
# Both agents appear in the name list at resolve time.
|
||||
self.assertIn("implementer", m.all_agent_names)
|
||||
self.assertIn("broken-agent", m.all_agent_names)
|
||||
# Valid agent loads fine.
|
||||
full = m.load_for_agent("implementer")
|
||||
self.assertIn("implementer", full.agents)
|
||||
# Broken bottle's agent raises at preflight.
|
||||
with self.assertRaises(ManifestError):
|
||||
m.load_for_agent("broken-agent")
|
||||
|
||||
|
||||
class TestStaleJsonDies(_ResolveCase):
|
||||
@@ -416,11 +390,11 @@ class TestNoManifestDies(_ResolveCase):
|
||||
self.assertEqual({}, dict(m.agents))
|
||||
|
||||
|
||||
class TestUnknownBottleReferenceDefersToBroken(_ResolveCase):
|
||||
"""An agent file naming a bottle that doesn't exist on disk is
|
||||
deferred into broken_agents; other agents still load."""
|
||||
class TestUnknownBottleReferenceFailsAtPreflight(_ResolveCase):
|
||||
"""An agent file naming a non-existent bottle appears in all_agent_names
|
||||
at resolve time; the error only surfaces when load_for_agent is called."""
|
||||
|
||||
def test_stray_bottle_reference_deferred(self):
|
||||
def test_stray_bottle_reference_fails_at_preflight(self):
|
||||
_write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV)
|
||||
_write(
|
||||
self.home_cb / "agents" / "stray.md",
|
||||
@@ -432,9 +406,15 @@ class TestUnknownBottleReferenceDefersToBroken(_ResolveCase):
|
||||
)
|
||||
_write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL)
|
||||
m = self.resolve()
|
||||
self.assertNotIn("stray", m.agents)
|
||||
self.assertIn("stray", m.broken_agents)
|
||||
self.assertIn("implementer", m.agents)
|
||||
# Both names visible at resolve time.
|
||||
self.assertIn("stray", m.all_agent_names)
|
||||
self.assertIn("implementer", m.all_agent_names)
|
||||
# Valid agent loads fine.
|
||||
full = m.load_for_agent("implementer")
|
||||
self.assertIn("implementer", full.agents)
|
||||
# Stray agent fails at preflight.
|
||||
with self.assertRaises(ManifestError):
|
||||
m.load_for_agent("stray")
|
||||
|
||||
|
||||
class TestFilenameValidation(_ResolveCase):
|
||||
@@ -448,9 +428,9 @@ class TestFilenameValidation(_ResolveCase):
|
||||
# This file should be skipped — capital letters not allowed.
|
||||
_write(self.home_cb / "agents" / "BadName.md", _AGENT_IMPL)
|
||||
m = self.resolve()
|
||||
self.assertIn("implementer", m.agents)
|
||||
self.assertNotIn("BadName", m.agents)
|
||||
self.assertNotIn("badname", m.agents)
|
||||
self.assertIn("implementer", m.all_agent_names)
|
||||
self.assertNotIn("BadName", m.all_agent_names)
|
||||
self.assertNotIn("badname", m.all_agent_names)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
Reference in New Issue
Block a user
Why are ALL the bottles and ALL the agents needed for a manifest?