From db3c714027480ce944b6977d4ad38a41cfb63ded Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 20 Jun 2026 02:45:33 +0000 Subject: [PATCH] refactor: scan filenames at resolve, parse only selected agent at preflight Manifest.resolve() now returns an empty-dict manifest with only directory paths recorded (home_md, cwd_md). No content is read from any .md file until load_for_agent() is called for a specific agent at preflight. - Manifest.from_md_dirs: scan-only, no frontmatter parsing - Manifest.load_for_agent: parses the selected agent file and its bottle chain; works on eager (from_json_obj) manifests too by returning self - Manifest.all_agent_names: scans filenames in lazy mode - backend._validate: calls load_for_agent and propagates upgraded spec - cli/info.py, cli/list.py, cli/start.py: use load_for_agent / all_agent_names - manifest_extends.py: reverted to original (no partial-resolve helpers) - manifest_loader.py: only scan_agent_names + load_bottle_chain_from_dir - Tests updated to call load_for_agent before accessing agents/bottles; test_md_agent_repos_deferred renamed to test_md_agent_repos_fails_at_preflight --- bot_bottle/backend/__init__.py | 23 +-- bot_bottle/cli/info.py | 5 +- bot_bottle/cli/list.py | 2 +- bot_bottle/manifest.py | 160 ++++++++++++++------- bot_bottle/manifest_extends.py | 85 +---------- bot_bottle/manifest_loader.py | 142 ++++++------------ tests/unit/test_manifest_agent_git_user.py | 19 +-- tests/unit/test_manifest_md_load.py | 110 ++++++-------- 8 files changed, 227 insertions(+), 319 deletions(-) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 9f04b99..8e0299f 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -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 diff --git a/bot_bottle/cli/info.py b/bot_bottle/cli/info.py index 9db0faf..9d2a62c 100644 --- a/bot_bottle/cli/info.py +++ b/bot_bottle/cli/info.py @@ -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) diff --git a/bot_bottle/cli/list.py b/bot_bottle/cli/list.py index c651da4..cb2149a 100644 --- a/bot_bottle/cli/list.py +++ b/bot_bottle/cli/list.py @@ -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 diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index 30b2493..7d4e622 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -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 - `/bottles/`, home agents from `/agents/`, - and (if `cwd_dir` is passed) cwd agents from - `/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) @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: diff --git a/bot_bottle/manifest_extends.py b/bot_bottle/manifest_extends.py index b4a0ab9..3432f64 100644 --- a/bot_bottle/manifest_extends.py +++ b/bot_bottle/manifest_extends.py @@ -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 diff --git a/bot_bottle/manifest_loader.py b/bot_bottle/manifest_loader.py index 6f5ac8e..6ce632a 100644 --- a/bot_bottle/manifest_loader.py +++ b/bot_bottle/manifest_loader.py @@ -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 `/*.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 `/*.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 `/*.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] diff --git a/tests/unit/test_manifest_agent_git_user.py b/tests/unit/test_manifest_agent_git_user.py index ebeb672..25d18f7 100644 --- a/tests/unit/test_manifest_agent_git_user.py +++ b/tests/unit/test_manifest_agent_git_user.py @@ -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) diff --git a/tests/unit/test_manifest_md_load.py b/tests/unit/test_manifest_md_load.py index 7636ef3..b2d0cdf 100644 --- a/tests/unit/test_manifest_md_load.py +++ b/tests/unit/test_manifest_md_load.py @@ -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__":