From 3375df3f52f6b644f1fdc3d343cd9a288b0dd9c5 Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 20 Jun 2026 02:02:28 +0000 Subject: [PATCH 1/8] feat: defer broken manifest parse errors to preflight Broken bottle/agent files no longer block the agent selector or prevent unrelated agents from loading. Per-file parse errors are collected in `Manifest.broken_agents`; the CLI selector includes them via `all_agent_names`, and the error surfaces only when the specific agent is selected and launch is attempted (in `require_agent`/`bottle_for`). Closes #236 --- bot_bottle/cli/start.py | 2 +- bot_bottle/manifest.py | 39 ++++++-- bot_bottle/manifest_extends.py | 83 ++++++++++++++++ bot_bottle/manifest_loader.py | 100 ++++++++++++++------ tests/unit/test_cli_start_selector.py | 1 + tests/unit/test_manifest_agent_git_user.py | 10 +- tests/unit/test_manifest_md_load.py | 104 ++++++++++++++++----- 7 files changed, 277 insertions(+), 62 deletions(-) diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index 55a47ac..087f129 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -67,7 +67,7 @@ def cmd_start(argv: list[str]) -> int: agent_name: str | None = args.name if agent_name is None: agent_name = tui.filter_select( - sorted(manifest.agents.keys()), + manifest.all_agent_names, title="Select agent", ) if agent_name is None: diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index 427e033..30b2493 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -193,6 +193,9 @@ 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) @classmethod def resolve(cls, cwd: str, *, missing_ok: bool = False) -> "Manifest": @@ -256,16 +259,23 @@ class Manifest: name collision. A `bottles/` subdir under `cwd_dir` is logged as a warning and ignored. + 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. + 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 = load_bottles_from_dir(bottles_dir) + bottles, broken_bottle_errors = load_bottles_from_dir(bottles_dir) bottle_names = set(bottles.keys()) agents_dir = home_dir / "agents" - agents = load_agents_from_dir(agents_dir, bottle_names, source="$HOME") + 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" @@ -281,12 +291,14 @@ class Manifest: f"(PRD 0011). Move them or delete." ) cwd_agents_dir = cwd_dir / "agents" - cwd_agents = load_agents_from_dir( - cwd_agents_dir, bottle_names, source="$CWD" + 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) + return cls(bottles=bottles, agents=agents, broken_agents=broken_agents) @classmethod def from_json_obj(cls, obj: object) -> "Manifest": @@ -311,10 +323,21 @@ class Manifest: } return cls(bottles=bottles, agents=agents) + @property + def all_agent_names(self) -> list[str]: + """Sorted list of all agent names, including broken ones. + + 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())) + 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] if self.has_agent(name): return available = ", ".join(self.agents.keys()) @@ -361,7 +384,11 @@ class Manifest: 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.""" + 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] 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 3432f64..b58e4d5 100644 --- a/bot_bottle/manifest_extends.py +++ b/bot_bottle/manifest_extends.py @@ -22,6 +22,25 @@ 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.""" + from .manifest import ManifestError + + 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]], @@ -210,3 +229,67 @@ 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, ManifestError + + 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 67d0e51..6f5ac8e 100644 --- a/bot_bottle/manifest_loader.py +++ b/bot_bottle/manifest_loader.py @@ -11,6 +11,7 @@ from .manifest_schema import ( validate_agent_frontmatter_keys, validate_bottle_frontmatter_keys, ) +from .manifest_util import ManifestError from .yaml_subset import YamlSubsetError, parse_frontmatter if TYPE_CHECKING: @@ -21,8 +22,6 @@ def check_stale_json(dir_path: Path, md_dir: Path, label: str) -> None: """Die if `/bot-bottle.json` exists but `md_dir` does not. The manifest format changed in PRD 0011 and we do not want to silently leave the JSON content unused.""" - from .manifest import ManifestError - legacy = dir_path / "bot-bottle.json" if legacy.is_file() and not md_dir.exists(): raise ManifestError( @@ -34,15 +33,20 @@ def check_stale_json(dir_path: Path, md_dir: Path, label: str) -> None: ) -def load_bottles_from_dir(bottles_dir: Path) -> dict[str, ManifestBottle]: +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}`. Missing dir returns an empty dict.""" - from .manifest import ManifestError - from .manifest_extends import resolve_bottles + `({name: Bottle}, {name: error})`. Missing dir returns empty dicts. + + 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 {} + return {}, {} for path in sorted(bottles_dir.glob("*.md")): name = entity_name_from_path(path) if name is None: @@ -54,12 +58,21 @@ def load_bottles_from_dir(bottles_dir: Path) -> dict[str, ManifestBottle]: try: fm, _body = parse_frontmatter(path.read_text()) except OSError as e: - raise ManifestError(f"could not read {path}: {e}") from e + broken[name] = ManifestError(f"could not read {path}: {e}") + continue except YamlSubsetError as e: - raise ManifestError(f"{path}: {e}") from e - validate_bottle_frontmatter_keys(path, fm.keys()) + 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 - return resolve_bottles(raws) + + good, resolve_broken = resolve_bottles_partial(raws) + broken.update(resolve_broken) + return good, broken def load_agents_from_dir( @@ -67,15 +80,26 @@ def load_agents_from_dir( bottle_names: set[str], *, source: str, # noqa: F841 — unused, but required by interface -) -> dict[str, ManifestAgent]: + 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}`. The Markdown body becomes the agent's prompt. - Missing dir returns an empty dict.""" - from .manifest import ManifestAgent, ManifestError + `({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] = {} if not agents_dir.is_dir(): - return out + return out, broken for path in sorted(agents_dir.glob("*.md")): name = entity_name_from_path(path) if name is None: @@ -87,19 +111,33 @@ def load_agents_from_dir( try: fm, body = parse_frontmatter(path.read_text()) except OSError as e: - raise ManifestError(f"could not read {path}: {e}") from e + broken[name] = ManifestError(f"could not read {path}: {e}") + continue except YamlSubsetError as e: - raise ManifestError(f"{path}: {e}") from e - validate_agent_frontmatter_keys(path, fm.keys()) - # Build the dict Agent.from_dict expects. The body becomes - # prompt; Claude Code passthrough fields stay in fm and get - # ignored by Agent.from_dict (reads bottle/skills/git-gate/prompt). - 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"] - out[name] = ManifestAgent.from_dict(name, agent_dict, bottle_names) - return out + 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 + + # 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]}" + ) + continue + + out[name] = agent + return out, broken diff --git a/tests/unit/test_cli_start_selector.py b/tests/unit/test_cli_start_selector.py index d0876a1..809d1a8 100644 --- a/tests/unit/test_cli_start_selector.py +++ b/tests/unit/test_cli_start_selector.py @@ -20,6 +20,7 @@ from bot_bottle.backend import ActiveAgent def _make_manifest(agent_names: list[str]): manifest = MagicMock() manifest.agents = {name: MagicMock() for name in agent_names} + manifest.all_agent_names = sorted(agent_names) return manifest diff --git a/tests/unit/test_manifest_agent_git_user.py b/tests/unit/test_manifest_agent_git_user.py index 3fb6c04..ebeb672 100644 --- a/tests/unit/test_manifest_agent_git_user.py +++ b/tests/unit/test_manifest_agent_git_user.py @@ -226,10 +226,16 @@ class TestAgentGitUserMdLoader(unittest.TestCase): m.git_identity_summary("impl"), ) - def test_md_agent_repos_dies(self): + 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.""" self._write("bottles/dev.md", _BOTTLE_DEV) self._write("agents/impl.md", _AGENT_WITH_REPOS) - msg = _error_message(Manifest.resolve, str(self.home)) + m = Manifest.resolve(str(self.home)) + self.assertNotIn("impl", m.agents) + self.assertIn("impl", m.broken_agents) + msg = str(m.broken_agents["impl"]) 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 4ec18dd..7636ef3 100644 --- a/tests/unit/test_manifest_md_load.py +++ b/tests/unit/test_manifest_md_load.py @@ -294,14 +294,15 @@ class TestManifestEntryPointParity(_ResolveCase): self.assertEqual("dev", manifest.agents["implementer"].bottle) -class TestUnknownAgentKeyDies(_ResolveCase): - """A typo'd / unknown frontmatter key on an agent file dies - rather than silently ignoring.""" +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.""" - def test_dies(self): + def test_broken_agent_deferred(self): _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) _write( - self.home_cb / "agents" / "implementer.md", + self.home_cb / "agents" / "bad.md", """ --- bottle: dev @@ -311,17 +312,58 @@ class TestUnknownAgentKeyDies(_ResolveCase): ... """, ) - with self.assertRaises(ManifestError): - self.resolve() + _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) - -class TestUnknownBottleKeyDies(_ResolveCase): - """A typo'd / unknown frontmatter key on a bottle file dies - rather than silently ignoring.""" - - def test_dies(self): + def test_broken_agent_appears_in_all_agent_names(self): + _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) _write( - self.home_cb / "bottles" / "dev.md", + self.home_cb / "agents" / "bad.md", + """ + --- + bottle: dev + skillz: [init-prd] + --- + """, + ) + _write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL) + m = self.resolve() + self.assertIn("bad", m.all_agent_names) + self.assertIn("implementer", m.all_agent_names) + + def test_broken_agent_raises_at_preflight(self): + _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) + _write( + self.home_cb / "agents" / "bad.md", + """ + --- + bottle: dev + skillz: [init-prd] + --- + """, + ) + m = self.resolve() + with self.assertRaises(ManifestError): + m.require_agent("bad") + with self.assertRaises(ManifestError): + m.bottle_for("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): + _write( + self.home_cb / "bottles" / "bad.md", """ --- credproxy: @@ -329,9 +371,24 @@ class TestUnknownBottleKeyDies(_ResolveCase): --- """, ) + _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) _write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL) - with self.assertRaises(ManifestError): - self.resolve() + _write( + self.home_cb / "agents" / "broken-agent.md", + """ + --- + bottle: bad + --- + """, + ) + 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) class TestStaleJsonDies(_ResolveCase): @@ -359,11 +416,11 @@ class TestNoManifestDies(_ResolveCase): self.assertEqual({}, dict(m.agents)) -class TestUnknownBottleReferenceDies(_ResolveCase): - """An agent file naming a bottle that doesn't exist on disk - dies with the existing "bottle not defined" error.""" +class TestUnknownBottleReferenceDefersToBroken(_ResolveCase): + """An agent file naming a bottle that doesn't exist on disk is + deferred into broken_agents; other agents still load.""" - def test_dies(self): + def test_stray_bottle_reference_deferred(self): _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) _write( self.home_cb / "agents" / "stray.md", @@ -373,8 +430,11 @@ class TestUnknownBottleReferenceDies(_ResolveCase): --- """, ) - with self.assertRaises(ManifestError): - self.resolve() + _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) class TestFilenameValidation(_ResolveCase): -- 2.52.0 From 996a260a98abe087b348ce21e2cd9cfe29a9d13d Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 20 Jun 2026 02:24:49 +0000 Subject: [PATCH 2/8] fix: resolve pyright reportUnusedImport in manifest_extends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Import ManifestError at module level from manifest_util (no circular dep) and remove the redundant local imports from function bodies that were shadowing it. ManifestBottle retains its local import pattern to avoid the circular manifest ↔ manifest_extends dependency. --- bot_bottle/manifest_extends.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot_bottle/manifest_extends.py b/bot_bottle/manifest_extends.py index b58e4d5..b4a0ab9 100644 --- a/bot_bottle/manifest_extends.py +++ b/bot_bottle/manifest_extends.py @@ -4,6 +4,8 @@ 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 @@ -31,8 +33,6 @@ def resolve_bottles_partial( collected in `broken` rather than raising, so unrelated bottles remain usable. Errors for parent bottles propagate to all children that extend them.""" - from .manifest import ManifestError - cache: dict[str, ManifestBottle] = {} broken: dict[str, ManifestError] = {} for name in raws: @@ -48,7 +48,7 @@ def _resolve_one_bottle( repos_cache: dict[str, dict[str, object]], seen: tuple[str, ...], ) -> ManifestBottle: - from .manifest import ManifestBottle, ManifestError + from .manifest import ManifestBottle if name in cache: return cache[name] @@ -239,7 +239,7 @@ def _resolve_one_bottle_partial( seen: tuple[str, ...], ) -> None: """Error-tolerant variant: on failure, adds to `broken` instead of raising.""" - from .manifest import ManifestBottle, ManifestError + from .manifest import ManifestBottle if name in cache or name in broken: return -- 2.52.0 From 3ccd09ed0d38fbfe217256fb6a967dfa57c1cec5 Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 20 Jun 2026 02:45:33 +0000 Subject: [PATCH 3/8] 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__": -- 2.52.0 From 2596c189540b9bf2a3c37f331bca3a4a9690ba43 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 22 Jun 2026 19:54:21 +0000 Subject: [PATCH 4/8] fix: load_for_agent always returns single-agent manifest Filter to exactly one agent and one bottle in both the lazy (md-dirs) and eager (from_json_obj) paths so the returned manifest invariant holds regardless of how the manifest was constructed. --- bot_bottle/manifest.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index 7d4e622..6fc581e 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -324,13 +324,20 @@ class Manifest: 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. + # Eager manifest (from_json_obj): data already parsed; filter to + # the one requested agent and its bottle so the returned manifest + # always contains exactly one agent and one bottle regardless of path. 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 + agent = self.agents[agent_name] + bottle_name = agent.bottle + return Manifest( + bottles={bottle_name: self.bottles[bottle_name]}, + agents={agent_name: agent}, + ) 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 -- 2.52.0 From 468ab8c29034384bb928f498bddc66be98a95310 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 22 Jun 2026 19:54:41 +0000 Subject: [PATCH 5/8] docs: clarify load_for_agent invariant in docstring --- bot_bottle/manifest.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index 6fc581e..776d8bc 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -317,12 +317,15 @@ class Manifest: 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. + """Return a Manifest containing exactly one agent 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.""" + In lazy mode (from resolve/from_md_dirs) the agent file and its + bottle chain are read from disk for the first time here. In eager + mode (from_json_obj) the data is already parsed; this just filters + down to the requested agent and its bottle. + + Always raises ManifestError if the agent is unknown or invalid. + Backends call this at preflight inside _validate.""" if self.home_md is None: # Eager manifest (from_json_obj): data already parsed; filter to # the one requested agent and its bottle so the returned manifest -- 2.52.0 From 294a6ed023f8e48240fe13a79ed35bce381c8034 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 23 Jun 2026 00:56:30 +0000 Subject: [PATCH 6/8] refactor(manifest): split Manifest into ManifestIndex + Manifest single-value type Manifest now holds exactly one agent and one effective bottle (with git_user overlay already applied). The old multi-agent/bottle collection is renamed ManifestIndex. BottleSpec.manifest starts as ManifestIndex from the CLI and becomes Manifest after _validate() calls load_for_agent(); all provisioning code downstream reads spec.manifest.agent / spec.manifest.bottle instead of indexing by name. --- bot_bottle/agent_provider.py | 2 +- bot_bottle/backend/__init__.py | 27 +-- bot_bottle/backend/docker/launch.py | 2 +- bot_bottle/backend/macos_container/launch.py | 2 +- bot_bottle/backend/resolve_common.py | 4 +- bot_bottle/backend/smolmachines/launch.py | 2 +- bot_bottle/cli/info.py | 10 +- bot_bottle/cli/list.py | 4 +- bot_bottle/cli/resume.py | 4 +- bot_bottle/cli/start.py | 4 +- bot_bottle/contrib/claude/agent_provider.py | 4 +- bot_bottle/contrib/codex/agent_provider.py | 4 +- bot_bottle/contrib/pi/agent_provider.py | 2 +- bot_bottle/env.py | 4 +- bot_bottle/manifest.py | 182 +++++++++--------- tests/fixtures.py | 14 +- .../test_macos_container_launch.py | 6 +- tests/integration/test_sandbox_escape.py | 4 +- .../test_sidecar_bundle_compose.py | 6 +- tests/integration/test_smolmachines_launch.py | 6 +- tests/unit/test_backend_prepare.py | 6 +- tests/unit/test_backend_workspace.py | 6 +- tests/unit/test_cli_start_selector.py | 4 +- tests/unit/test_compose.py | 6 +- tests/unit/test_contrib_claude_provider.py | 6 +- tests/unit/test_contrib_codex_provider.py | 6 +- tests/unit/test_contrib_pi_provider.py | 6 +- tests/unit/test_docker_launch_teardown.py | 9 +- tests/unit/test_docker_provision_git_user.py | 6 +- tests/unit/test_egress.py | 8 +- tests/unit/test_git_gate.py | 4 +- tests/unit/test_manifest_agent_git_user.py | 79 +++++--- tests/unit/test_manifest_egress.py | 22 +-- tests/unit/test_manifest_extends.py | 4 +- tests/unit/test_manifest_git.py | 76 ++++---- tests/unit/test_manifest_git_user.py | 20 +- tests/unit/test_manifest_md_load.py | 55 +++--- tests/unit/test_manifest_runtime.py | 6 +- tests/unit/test_plan_print_parity.py | 6 +- tests/unit/test_provision_git.py | 4 +- tests/unit/test_smolmachines_provision.py | 6 +- 41 files changed, 330 insertions(+), 308 deletions(-) diff --git a/bot_bottle/agent_provider.py b/bot_bottle/agent_provider.py index 791604e..20f0bb7 100644 --- a/bot_bottle/agent_provider.py +++ b/bot_bottle/agent_provider.py @@ -240,7 +240,7 @@ class AgentProvider(ABC): BottleBackend.provision_workspace against the running bottle.""" from .log import info - manifest_bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name) + manifest_bottle = plan.spec.manifest.bottle if manifest_bottle.git: from .git_gate import GIT_GATE_HOSTNAME, git_gate_render_gitconfig gate_host = getattr(plan, "git_gate_insteadof_host", GIT_GATE_HOSTNAME) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 8e0299f..ad32351 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -45,7 +45,7 @@ from ..agent_provider import AgentProvisionPlan, get_provider, build_agent_provi from ..egress import EgressPlan from ..git_gate import GitGatePlan from ..log import die, info -from ..manifest import Manifest +from ..manifest import Manifest, ManifestIndex from ..supervise import SupervisePlan from ..util import expand_tilde from ..env import resolve_env, ResolvedEnv @@ -61,7 +61,7 @@ class BottleSpec: Resolved values (image names, container name, scratch paths, runsc availability) live on the plan, not the spec.""" - manifest: Manifest + manifest: ManifestIndex | Manifest agent_name: str copy_cwd: bool user_cwd: str @@ -112,9 +112,9 @@ class BottlePlan(ABC): """Render the y/N preflight summary to stderr.""" del remote_control spec = self.spec - manifest = spec.manifest - agent = manifest.agents[spec.agent_name] - bottle = manifest.bottle_for(spec.agent_name) + manifest = spec.manifest # type: ignore[assignment] + agent = manifest.agent + bottle = manifest.bottle env_names = visible_agent_env_names( sorted( @@ -131,7 +131,7 @@ class BottlePlan(ABC): print_multi("skills ", list(agent.skills)) info(f"bottle : {agent.bottle}") - identity = manifest.git_identity_summary(spec.agent_name) + identity = manifest.git_identity_summary() if identity: info(f" git identity : {identity}") @@ -293,11 +293,11 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): self._preflight() - manifest = spec.manifest - manifest_bottle = manifest.bottle_for(spec.agent_name) + manifest = spec.manifest # type: ignore[assignment] + manifest_bottle = manifest.bottle manifest_agent_provider = manifest_bottle.agent_provider agent_provider = get_provider(manifest_agent_provider.template) - resolved_env = resolve_env(manifest, spec.agent_name) + resolved_env = resolve_env(manifest) workspace = workspace_plan(spec, guest_home=agent_provider.guest_home) slug = mint_slug(spec) @@ -364,9 +364,9 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): 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) + manifest = spec.manifest.load_for_agent(spec.agent_name) # type: ignore[union-attr] spec = replace(spec, manifest=manifest) - agent = manifest.agents[spec.agent_name] + agent = manifest.agent self._validate_skills(agent.skills) self._validate_agent_provider_dockerfile(spec) return spec @@ -384,7 +384,8 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): ) def _validate_agent_provider_dockerfile(self, spec: BottleSpec) -> None: - bottle = spec.manifest.bottle_for(spec.agent_name) + manifest = spec.manifest # type: ignore[assignment] + bottle = manifest.bottle dockerfile = bottle.agent_provider.dockerfile if not dockerfile: return @@ -394,7 +395,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): if not path.is_file(): die( f"agent_provider.dockerfile for bottle " - f"'{spec.manifest.agents[spec.agent_name].bottle}' not found: {path}" + f"'{manifest.agent.bottle}' not found: {path}" ) @abstractmethod diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index c753e1f..f937b30 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -75,7 +75,7 @@ def launch( Teardown on exit.""" stack = ExitStack() - _bottle_for_revoke = plan.spec.manifest.bottle_for(plan.spec.agent_name) + _bottle_for_revoke = plan.spec.manifest.bottle _git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) def teardown() -> None: diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index a877bd7..9db7411 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -68,7 +68,7 @@ def launch( ) -> Generator[MacosContainerBottle, None, None]: """Build, run, provision, and yield an Apple Container bottle.""" stack = ExitStack() - bottle_for_revoke = plan.spec.manifest.bottle_for(plan.spec.agent_name) + bottle_for_revoke = plan.spec.manifest.bottle git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) def teardown() -> None: diff --git a/bot_bottle/backend/resolve_common.py b/bot_bottle/backend/resolve_common.py index 549a466..affb345 100644 --- a/bot_bottle/backend/resolve_common.py +++ b/bot_bottle/backend/resolve_common.py @@ -69,8 +69,8 @@ def write_launch_metadata( def prepare_agent_state_dir(slug: str, spec: BottleSpec) -> tuple[Path, Path]: """Create the agent state subdir, write the prompt file. Returns (agent_dir, prompt_file).""" - manifest = spec.manifest - agent = manifest.agents[spec.agent_name] + manifest = spec.manifest # type: ignore[assignment] + agent = manifest.agent agent_dir = agent_state_dir(slug) agent_dir.mkdir(parents=True, exist_ok=True) prompt_file = agent_dir / "prompt.txt" diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index 20c614b..e16b611 100644 --- a/bot_bottle/backend/smolmachines/launch.py +++ b/bot_bottle/backend/smolmachines/launch.py @@ -130,7 +130,7 @@ def _teardown_smolmachines( except BaseException as exc: # noqa: W0718 — teardown must not fail teardown_exc = exc warn(f"smolmachines teardown failed: {exc!r}") - bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name) + bottle = plan.spec.manifest.bottle revoke_git_gate_provisioned_keys(bottle, git_gate_state_dir(plan.slug)) if teardown_exc is not None: raise teardown_exc diff --git a/bot_bottle/cli/info.py b/bot_bottle/cli/info.py index 9d2a62c..61b8945 100644 --- a/bot_bottle/cli/info.py +++ b/bot_bottle/cli/info.py @@ -5,7 +5,7 @@ from __future__ import annotations import argparse from ..log import info -from ..manifest import Manifest +from ..manifest import ManifestIndex from ._common import PROG, USER_CWD @@ -14,12 +14,12 @@ def cmd_info(argv: list[str]) -> int: parser.add_argument("name", help="agent name defined in bot-bottle.json") args = parser.parse_args(argv) - names = Manifest.resolve(USER_CWD) + names = ManifestIndex.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) + agent = manifest.agent + bottle = manifest.bottle env_names = list(bottle.env.keys()) prompt_first_line = agent.prompt.splitlines()[0] if agent.prompt else "" @@ -32,7 +32,7 @@ def cmd_info(argv: list[str]) -> int: f"first line: {prompt_first_line or '(empty)'}" ) info(f"bottle : {agent.bottle}") - identity = manifest.git_identity_summary(args.name) + identity = manifest.git_identity_summary() if identity: info(f" git identity : {identity}") if bottle.git: diff --git a/bot_bottle/cli/list.py b/bot_bottle/cli/list.py index cb2149a..b04e772 100644 --- a/bot_bottle/cli/list.py +++ b/bot_bottle/cli/list.py @@ -7,7 +7,7 @@ import os import sys from ..backend import enumerate_active_agents -from ..manifest import Manifest +from ..manifest import ManifestIndex from ._common import PROG, USER_CWD _ANSI_COLOR_CODES: dict[str, str] = { @@ -40,7 +40,7 @@ def cmd_list(argv: list[str]) -> int: args = parser.parse_args(argv) if args.scope == "available": - manifest = Manifest.resolve(USER_CWD) + manifest = ManifestIndex.resolve(USER_CWD) for name in manifest.all_agent_names: print(name) return 0 diff --git a/bot_bottle/cli/resume.py b/bot_bottle/cli/resume.py index ee77088..557c293 100644 --- a/bot_bottle/cli/resume.py +++ b/bot_bottle/cli/resume.py @@ -20,7 +20,7 @@ import argparse from ..backend import BottleSpec from ..bottle_state import read_metadata from ..log import die -from ..manifest import Manifest +from ..manifest import ManifestIndex from ._common import PROG, USER_CWD from .start import _launch_bottle @@ -42,7 +42,7 @@ def cmd_resume(argv: list[str]) -> int: f"check ~/.bot-bottle/state/ or run `cli.py start` to create a new bottle" ) - manifest = Manifest.resolve(USER_CWD) + manifest = ManifestIndex.resolve(USER_CWD) manifest.require_agent(metadata.agent_name) spec = BottleSpec( diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index 087f129..c752cfc 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -33,7 +33,7 @@ from ..bottle_state import ( ) # from ..backend.docker.capability_apply import snapshot_transcript from ..log import info -from ..manifest import Manifest +from ..manifest import ManifestIndex from ._common import PROG, USER_CWD, read_tty_line from . import tui @@ -62,7 +62,7 @@ def cmd_start(argv: list[str]) -> int: dry_run = args.dry_run or os.environ.get("BOT_BOTTLE_DRY_RUN") == "1" - manifest = Manifest.resolve(USER_CWD) + manifest = ManifestIndex.resolve(USER_CWD) agent_name: str | None = args.name if agent_name is None: diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index 4ef1403..add66a2 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -211,7 +211,7 @@ class ClaudeAgentProvider(AgentProvider): when the agent has no skills.""" from ...backend.util import host_skill_dir - agent = plan.spec.manifest.agents[plan.spec.agent_name] + agent = plan.spec.manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) @@ -240,7 +240,7 @@ class ClaudeAgentProvider(AgentProvider): f"chown node:node {prompt_path} && chmod 600 {prompt_path}", user="root", ) - agent = plan.spec.manifest.agents[plan.spec.agent_name] + agent = plan.spec.manifest.agent return prompt_path if plan.agent_provision.has_prompt or agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 7207eb1..8b5f485 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -177,7 +177,7 @@ class CodexAgentProvider(AgentProvider): skills.""" from ...backend.util import host_skill_dir - agent = plan.spec.manifest.agents[plan.spec.agent_name] + agent = plan.spec.manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) @@ -206,7 +206,7 @@ class CodexAgentProvider(AgentProvider): f"chown node:node {prompt_path} && chmod 600 {prompt_path}", user="root", ) - agent = plan.spec.manifest.agents[plan.spec.agent_name] + agent = plan.spec.manifest.agent return prompt_path if plan.agent_provision.has_prompt or agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: diff --git a/bot_bottle/contrib/pi/agent_provider.py b/bot_bottle/contrib/pi/agent_provider.py index 874e82f..83d498d 100644 --- a/bot_bottle/contrib/pi/agent_provider.py +++ b/bot_bottle/contrib/pi/agent_provider.py @@ -232,7 +232,7 @@ class PiAgentProvider(AgentProvider): def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None: from ...backend.util import host_skill_dir - agent = plan.spec.manifest.agents[plan.spec.agent_name] + agent = plan.spec.manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) diff --git a/bot_bottle/env.py b/bot_bottle/env.py index fe4362d..767ad04 100644 --- a/bot_bottle/env.py +++ b/bot_bottle/env.py @@ -114,7 +114,7 @@ def _read_secret_silent(name: str, prompt_body: str) -> str: return value -def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv: +def resolve_env(manifest: Manifest) -> ResolvedEnv: """Iterate the agent's env entries: - secret: prompt at runtime; carry value in forwarded - interpolated: read $HOST_VAR from os.environ; carry value in forwarded @@ -124,7 +124,7 @@ def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv: backend injects forwarded values via its launcher's env parameter.""" forwarded: dict[str, str] = {} literals: dict[str, str] = {} - bottle = manifest.bottle_for(agent) + bottle = manifest.bottle for name, raw in bottle.env.items(): if not name: continue diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index 776d8bc..224e7ea 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -36,10 +36,23 @@ Bottles can ONLY live under $HOME. A bottles/ dir under $CWD is a warn at load time and contributes nothing. The trust boundary is expressed as filesystem layout rather than resolver logic. -Validation runs once at load. Manifest.from_json_obj is preserved -as a programmatic entry point (used by tests) that takes a dict -with the same field names — useful for building manifests without -on-disk files. +Two types are exported: + + ManifestIndex — the multi-agent/bottle collection returned by + resolve() and from_json_obj(). Used for agent + selection (all_agent_names), validation + (require_agent), and lazy loading (load_for_agent). + This is the pre-preflight form. + + Manifest — a single-agent/bottle value type holding exactly + one agent: ManifestAgent and one bottle: + ManifestBottle (with the agent's git-gate.user + already overlaid). Returned by load_for_agent(). + This is the post-preflight form passed to backends. + +ManifestIndex.from_json_obj is preserved as a programmatic entry +point (used by tests) that takes a dict with the same field names — +useful for building manifests without on-disk files. """ from __future__ import annotations @@ -71,6 +84,7 @@ __all__ = [ "ManifestEgressConfig", "ManifestAgent", "ManifestBottle", + "ManifestIndex", "Manifest", ] @@ -189,18 +203,64 @@ class ManifestBottle: ) +def _merge_git_user( + agent_user: ManifestGitUser, base_user: ManifestGitUser +) -> ManifestGitUser: + """Merge the agent's git.user over the bottle's, agent-wins-on-non-empty.""" + if agent_user.is_empty(): + return base_user + return ManifestGitUser( + name=agent_user.name or base_user.name, + email=agent_user.email or base_user.email, + ) + + @dataclass(frozen=True) class Manifest: + """Single-agent/bottle value type. Returned by ManifestIndex.load_for_agent(). + + `bottle` is the effective bottle with the agent's git-gate.user already + overlaid per-field (agent wins on non-empty). Backends and provisioners + use this directly — no agent_name lookup needed.""" + + agent: ManifestAgent + bottle: ManifestBottle + + def git_identity_summary(self) -> str | None: + """One-line effective git identity with per-field provenance, e.g. + `name=claude (agent), email=eric@dideric.is (bottle)`. + Returns None when neither agent nor bottle sets an identity.""" + over = self.agent.git_user # agent's declared git_user (pre-merge) + merged = self.bottle.git_user # effective git_user (post-merge) + if merged.is_empty(): + return None + parts: list[str] = [] + if merged.name: + parts.append(f"name={merged.name} ({'agent' if over.name else 'bottle'})") + if merged.email: + parts.append(f"email={merged.email} ({'agent' if over.email else 'bottle'})") + return ", ".join(parts) + + +@dataclass(frozen=True) +class ManifestIndex: + """Multi-agent/bottle collection. The pre-preflight form. + + In lazy mode (from resolve()/from_md_dirs()) only filenames are scanned; + no file content is read. In eager mode (from from_json_obj()) all agents + and bottles are pre-parsed. Call load_for_agent() to get a single-value + Manifest ready for backend use.""" + bottles: Mapping[str, ManifestBottle] agents: Mapping[str, ManifestAgent] - # Set by from_md_dirs; empty in from_json_obj (test/programmatic) mode. + # Set by from_md_dirs; None 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": - """Walk the per-file manifest tree and build a Manifest. + def resolve(cls, cwd: str, *, missing_ok: bool = False) -> "ManifestIndex": + """Walk the per-file manifest tree and build a ManifestIndex. Layout (PRD 0011): $HOME/.bot-bottle/bottles/.md — bottles (home-only) @@ -213,7 +273,7 @@ class Manifest: boundary. If `missing_ok` is true, a missing `$HOME/.bot-bottle/` - returns an empty manifest instead of dying. This is for + returns an empty index instead of dying. This is for passive UI surfaces like the dashboard, which can still monitor already-running agents without launch config. @@ -252,15 +312,15 @@ class Manifest: cls, home_dir: Path, cwd_dir: Path | None, - ) -> "Manifest": - """Return a names-only Manifest. No file content is read; only + ) -> "ManifestIndex": + """Return a names-only ManifestIndex. No file content is read; only filenames are scanned for the agent selector. Full parsing happens later, per-agent, via `load_for_agent`. 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 + Used by tests to build a ManifestIndex from fixture directories without touching `os.environ`.""" if cwd_dir is not None: stale_bottles = cwd_dir / "bottles" @@ -278,8 +338,8 @@ class Manifest: return cls(bottles={}, agents={}, home_md=home_dir, cwd_md=cwd_dir) @classmethod - def from_json_obj(cls, obj: object) -> "Manifest": - """Validate and build a Manifest from a raw JSON-like dict.""" + def from_json_obj(cls, obj: object) -> "ManifestIndex": + """Validate and build a ManifestIndex from a raw JSON-like dict.""" d = as_json_object(obj, "manifest") raw_bottles_obj = _section_dict(d.get("bottles"), "manifest 'bottles'") raw_agents = _section_dict(d.get("agents"), "manifest 'agents'") @@ -317,30 +377,33 @@ class Manifest: return sorted(self.agents.keys()) def load_for_agent(self, agent_name: str) -> "Manifest": - """Return a Manifest containing exactly one agent and its bottle. + """Parse the named agent and its bottle; return a single-value Manifest. In lazy mode (from resolve/from_md_dirs) the agent file and its bottle chain are read from disk for the first time here. In eager mode (from_json_obj) the data is already parsed; this just filters down to the requested agent and its bottle. + The returned Manifest.bottle has the agent's git-gate.user already + overlaid (agent wins on non-empty, per-field). + Always raises ManifestError if the agent is unknown or invalid. Backends call this at preflight inside _validate.""" if self.home_md is None: # Eager manifest (from_json_obj): data already parsed; filter to - # the one requested agent and its bottle so the returned manifest - # always contains exactly one agent and one bottle regardless of path. + # the one requested agent and its bottle so the returned Manifest + # always holds exactly one agent and one bottle regardless of path. 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}" ) agent = self.agents[agent_name] - bottle_name = agent.bottle - return Manifest( - bottles={bottle_name: self.bottles[bottle_name]}, - agents={agent_name: agent}, - ) + raw_bottle = self.bottles[agent.bottle] + merged = _merge_git_user(agent.git_user, raw_bottle.git_user) + bottle = raw_bottle if merged == raw_bottle.git_user else replace(raw_bottle, git_user=merged) + return Manifest(agent=agent, bottle=bottle) + 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 @@ -350,15 +413,15 @@ class Manifest: 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} + merged_agents = {**home_agents, **cwd_agents} - if agent_name not in merged: - available = ", ".join(sorted(merged.keys())) or "(none)" + if agent_name not in merged_agents: + available = ", ".join(sorted(merged_agents.keys())) or "(none)" raise ManifestError( f"agent '{agent_name}' not defined. Available: {available}" ) - agent_path = merged[agent_name] + agent_path = merged_agents[agent_name] try: fm, body = parse_frontmatter(agent_path.read_text()) except OSError as e: @@ -377,7 +440,7 @@ class Manifest: # Load the bottle chain (may raise ManifestError). bottles_dir = self.home_md / "bottles" - bottle = load_bottle_chain_from_dir(bottle_name, bottles_dir) + raw_bottle = load_bottle_chain_from_dir(bottle_name, bottles_dir) # Build and validate the full ManifestAgent. agent_dict: dict[str, object] = { @@ -389,12 +452,9 @@ class Manifest: 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, - ) + merged_user = _merge_git_user(agent.git_user, raw_bottle.git_user) + bottle = raw_bottle if merged_user == raw_bottle.git_user else replace(raw_bottle, git_user=merged_user) + return Manifest(agent=agent, bottle=bottle) def has_agent(self, name: str) -> bool: return name in self.agents @@ -418,61 +478,3 @@ class Manifest: raise ManifestError( f"agent '{name}' not defined. Available: {available}" ) - - def has_bottle(self, name: str) -> bool: - return name in self.bottles - - def require_bottle(self, name: str) -> None: - if self.has_bottle(name): - return - available = ", ".join(self.bottles.keys()) - if available: - raise ManifestError( - f"bottle '{name}' not defined in bot-bottle.json. " - f"Available bottles: {available}" - ) - raise ManifestError(f"bottle '{name}' not defined in bot-bottle.json (no bottles defined).") - - def _effective_git_user(self, agent_name: str) -> ManifestGitUser: - """Merge the agent's git.user over the referenced bottle's, - per-field, agent-wins-on-non-empty (issue #94). Same overlay - the `extends:` resolver applies between bottles - (`_merge_bottles`).""" - agent = self.agents[agent_name] - base = self.bottles[agent.bottle].git_user - over = agent.git_user - if over.is_empty(): - return base - return ManifestGitUser( - name=over.name or base.name, - email=over.email or base.email, - ) - - 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 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.""" - bottle = self.bottles[self.agents[agent_name].bottle] - merged = self._effective_git_user(agent_name) - if merged == bottle.git_user: - return bottle - return replace(bottle, git_user=merged) - - def git_identity_summary(self, agent_name: str) -> str | None: - """One-line effective git identity with per-field provenance - for launch summaries, e.g. - `name=claude (agent), email=eric@dideric.is (bottle)`. - Returns None when neither agent nor bottle sets an identity.""" - over = self.agents[agent_name].git_user - merged = self._effective_git_user(agent_name) - if merged.is_empty(): - return None - parts: list[str] = [] - if merged.name: - parts.append(f"name={merged.name} ({'agent' if over.name else 'bottle'})") - if merged.email: - parts.append(f"email={merged.email} ({'agent' if over.email else 'bottle'})") - return ", ".join(parts) diff --git a/tests/fixtures.py b/tests/fixtures.py index 6bd1d83..38a4e2b 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -10,7 +10,7 @@ import tempfile from pathlib import Path from typing import Any, Callable -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex def fixture_minimal_dict() -> dict[str, Any]: @@ -62,16 +62,16 @@ def fixture_with_git_dict() -> dict[str, Any]: } -def fixture_minimal() -> Manifest: - return Manifest.from_json_obj(fixture_minimal_dict()) +def fixture_minimal() -> ManifestIndex: + return ManifestIndex.from_json_obj(fixture_minimal_dict()) -def fixture_with_egress() -> Manifest: - return Manifest.from_json_obj(fixture_with_egress_dict()) +def fixture_with_egress() -> ManifestIndex: + return ManifestIndex.from_json_obj(fixture_with_egress_dict()) -def fixture_with_git() -> Manifest: - return Manifest.from_json_obj(fixture_with_git_dict()) +def fixture_with_git() -> ManifestIndex: + return ManifestIndex.from_json_obj(fixture_with_git_dict()) def write_fixture(fn: Callable[[], dict[str, Any]]) -> Path: diff --git a/tests/integration/test_macos_container_launch.py b/tests/integration/test_macos_container_launch.py index 9ce384f..71678d5 100644 --- a/tests/integration/test_macos_container_launch.py +++ b/tests/integration/test_macos_container_launch.py @@ -29,7 +29,7 @@ from bot_bottle.backend.macos_container.util import ( dns_server as _container_dns_server, is_available as _container_available, ) -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex _AGENT_PROMPT = "You are a launch smoke-test agent. Be brief." @@ -52,8 +52,8 @@ def _minimal_agent_dockerfile(path: Path) -> None: ) -def _minimal_manifest(dockerfile: Path) -> Manifest: - return Manifest.from_json_obj({ +def _minimal_manifest(dockerfile: Path) -> ManifestIndex: + return ManifestIndex.from_json_obj({ "bottles": { "dev": { "agent_provider": { diff --git a/tests/integration/test_sandbox_escape.py b/tests/integration/test_sandbox_escape.py index e8fc97d..332ab2e 100644 --- a/tests/integration/test_sandbox_escape.py +++ b/tests/integration/test_sandbox_escape.py @@ -31,7 +31,7 @@ from pathlib import Path from bot_bottle.backend import BottleSpec, get_bottle_backend from bot_bottle.bottle_state import cleanup_state -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from tests._docker import skip_unless_docker @@ -101,7 +101,7 @@ class TestSandboxEscape(unittest.TestCase): cls._key_path.write_text("placeholder\n") cls._key_path.chmod(0o600) - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": { "dev": { # Three fake secrets — different shapes — land diff --git a/tests/integration/test_sidecar_bundle_compose.py b/tests/integration/test_sidecar_bundle_compose.py index 4d6b9cd..b57600e 100644 --- a/tests/integration/test_sidecar_bundle_compose.py +++ b/tests/integration/test_sidecar_bundle_compose.py @@ -22,15 +22,15 @@ from pathlib import Path from unittest.mock import patch from bot_bottle.backend import BottleSpec, get_bottle_backend -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from tests._docker import skip_unless_docker -def _manifest() -> Manifest: +def _manifest() -> ManifestIndex: """Bottle with supervise on so the bundle exercises egress + supervise. Git is off because a meaningful git-gate test needs a real upstream and SSH keys — out of scope for a bundle smoke.""" - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": { "dev": { "supervise": True, diff --git a/tests/integration/test_smolmachines_launch.py b/tests/integration/test_smolmachines_launch.py index 706eb3b..2f5606f 100644 --- a/tests/integration/test_smolmachines_launch.py +++ b/tests/integration/test_smolmachines_launch.py @@ -35,15 +35,15 @@ from pathlib import Path from bot_bottle.backend import BottleSpec, get_bottle_backend from bot_bottle.backend.smolmachines.smolvm import is_available as _smolvm_available -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from tests._docker import skip_unless_docker _AGENT_PROMPT = "You are demo. Be brief." -def _minimal_manifest() -> Manifest: - return Manifest.from_json_obj({ +def _minimal_manifest() -> ManifestIndex: + return ManifestIndex.from_json_obj({ "bottles": { "dev": { "egress": { diff --git a/tests/unit/test_backend_prepare.py b/tests/unit/test_backend_prepare.py index ee673a1..013e5bb 100644 --- a/tests/unit/test_backend_prepare.py +++ b/tests/unit/test_backend_prepare.py @@ -18,11 +18,11 @@ from bot_bottle.backend import BottleSpec from bot_bottle.backend.docker import DockerBottleBackend from bot_bottle.backend.resolve_common import mint_slug from bot_bottle.backend.smolmachines import SmolmachinesBottleBackend -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex -def _manifest() -> Manifest: - return Manifest.from_json_obj({ +def _manifest() -> ManifestIndex: + return ManifestIndex.from_json_obj({ "bottles": { "dev": { "env": { diff --git a/tests/unit/test_backend_workspace.py b/tests/unit/test_backend_workspace.py index bcbfddf..5b7cfcd 100644 --- a/tests/unit/test_backend_workspace.py +++ b/tests/unit/test_backend_workspace.py @@ -17,11 +17,11 @@ from bot_bottle import supervise from bot_bottle.backend import Bottle, BottleSpec, ExecResult from bot_bottle.backend.docker import DockerBottleBackend from bot_bottle.backend.smolmachines import SmolmachinesBottleBackend -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex -def _manifest() -> Manifest: - return Manifest.from_json_obj({ +def _manifest() -> ManifestIndex: + return ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": { "demo": { diff --git a/tests/unit/test_cli_start_selector.py b/tests/unit/test_cli_start_selector.py index 809d1a8..928652f 100644 --- a/tests/unit/test_cli_start_selector.py +++ b/tests/unit/test_cli_start_selector.py @@ -31,7 +31,7 @@ class TestCmdStartSelector(unittest.TestCase): # Stub Manifest.resolve so no on-disk manifest is needed. self._manifest = _make_manifest(["researcher", "implementer"]) self._resolve_patch = patch( - "bot_bottle.cli.start.Manifest.resolve", + "bot_bottle.cli.start.ManifestIndex.resolve", return_value=self._manifest, ) self._resolve_patch.start() @@ -150,7 +150,7 @@ class TestCmdStartLabelCollision(unittest.TestCase): def setUp(self): self._manifest = _make_manifest(["researcher"]) - patch("bot_bottle.cli.start.Manifest.resolve", return_value=self._manifest).start() + patch("bot_bottle.cli.start.ManifestIndex.resolve", return_value=self._manifest).start() self._launch_mock = patch( "bot_bottle.cli.start._launch_bottle", return_value=0, ).start() diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index 082f5ab..dafe3ef 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -31,7 +31,7 @@ from bot_bottle.egress import ( EgressRoute, ) from bot_bottle.git_gate import GitGatePlan, GitGateUpstream -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from bot_bottle.supervise import SupervisePlan @@ -40,7 +40,7 @@ STAGE = Path("/tmp/cb-stage") STATE = Path("/tmp/cb-state") -def _manifest(*, supervise: bool, with_git: bool, with_egress: bool) -> Manifest: +def _manifest(*, supervise: bool, with_git: bool, with_egress: bool) -> ManifestIndex: """Minimal manifest with the toggles the chunk-1 matrix needs. The renderer only reads from the plan, not the manifest, so this is just here to back BottleSpec.""" @@ -61,7 +61,7 @@ def _manifest(*, supervise: bool, with_git: bool, with_egress: bool) -> Manifest "auth": {"scheme": "Bearer", "token_ref": "TOK"}, }], } - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": {"dev": bottle}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) diff --git a/tests/unit/test_contrib_claude_provider.py b/tests/unit/test_contrib_claude_provider.py index 42f034a..c8ce4c0 100644 --- a/tests/unit/test_contrib_claude_provider.py +++ b/tests/unit/test_contrib_claude_provider.py @@ -24,7 +24,7 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.contrib.claude.agent_provider import ClaudeAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import Manifest, ManifestIndex from bot_bottle.supervise import SupervisePlan @@ -55,7 +55,7 @@ def _plan( bottle_json: dict = {"agent_provider": {"template": "claude"}} # type: ignore if supervise: bottle_json["supervise"] = True - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": { "demo": { @@ -64,7 +64,7 @@ def _plan( "bottle": "dev", }, }, - }) + }).load_for_agent("demo") spec = BottleSpec( manifest=manifest, agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", diff --git a/tests/unit/test_contrib_codex_provider.py b/tests/unit/test_contrib_codex_provider.py index 65a8ee2..4e5411c 100644 --- a/tests/unit/test_contrib_codex_provider.py +++ b/tests/unit/test_contrib_codex_provider.py @@ -24,7 +24,7 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.contrib.codex.agent_provider import CodexAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import Manifest, ManifestIndex from bot_bottle.supervise import SupervisePlan @@ -55,7 +55,7 @@ def _plan( bottle_json: dict = {"agent_provider": {"template": "codex"}} # type: ignore if supervise: bottle_json["supervise"] = True - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": { "demo": { @@ -64,7 +64,7 @@ def _plan( "bottle": "dev", }, }, - }) + }).load_for_agent("demo") spec = BottleSpec( manifest=manifest, agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", diff --git a/tests/unit/test_contrib_pi_provider.py b/tests/unit/test_contrib_pi_provider.py index ade0db2..dd0a88c 100644 --- a/tests/unit/test_contrib_pi_provider.py +++ b/tests/unit/test_contrib_pi_provider.py @@ -16,7 +16,7 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.contrib.pi.agent_provider import PiAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import Manifest, ManifestIndex _URL = "http://supervise:9100/" @@ -43,7 +43,7 @@ def _plan( skills: list[str] | None = None, agent_provision: AgentProvisionPlan | None = None, ) -> DockerBottlePlan: - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": {"dev": {"agent_provider": {"template": "pi"}}}, "agents": { "demo": { @@ -52,7 +52,7 @@ def _plan( "bottle": "dev", }, }, - }) + }).load_for_agent("demo") spec = BottleSpec( manifest=manifest, agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", diff --git a/tests/unit/test_docker_launch_teardown.py b/tests/unit/test_docker_launch_teardown.py index 58b9c83..2a765c9 100644 --- a/tests/unit/test_docker_launch_teardown.py +++ b/tests/unit/test_docker_launch_teardown.py @@ -21,14 +21,17 @@ from bot_bottle.backend.docker import launch as launch_mod from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex + + +from bot_bottle.manifest import Manifest, ManifestIndex def _manifest() -> Manifest: - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }) + }).load_for_agent("demo") def _plan(tmp: str) -> DockerBottlePlan: diff --git a/tests/unit/test_docker_provision_git_user.py b/tests/unit/test_docker_provision_git_user.py index 5513fc5..6c1220c 100644 --- a/tests/unit/test_docker_provision_git_user.py +++ b/tests/unit/test_docker_provision_git_user.py @@ -21,7 +21,7 @@ from bot_bottle.backend import Bottle, BottleSpec, ExecResult from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import Manifest, ManifestIndex class _Provider(AgentProvider): @@ -51,10 +51,10 @@ def _plan(*, git_user: dict | None = None, # type: ignore bottle_json: dict = {} # type: ignore if git_user is not None: bottle_json["git-gate"] = {"user": git_user} - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }) + }).load_for_agent("demo") spec = BottleSpec( manifest=manifest, agent_name="demo", copy_cwd=copy_cwd, user_cwd=user_cwd, diff --git a/tests/unit/test_egress.py b/tests/unit/test_egress.py index 36c7bb9..05ea426 100644 --- a/tests/unit/test_egress.py +++ b/tests/unit/test_egress.py @@ -13,12 +13,12 @@ from bot_bottle.egress import ( egress_token_env_map, ) from bot_bottle.log import Die -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from bot_bottle.yaml_subset import parse_yaml_subset def _bottle(routes): # type: ignore - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"routes": routes}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }).bottles["dev"] @@ -362,9 +362,9 @@ class TestRenderRoutes(unittest.TestCase): self.assertEqual("x.example", cfg.routes[0].host) def test_log_via_manifest_flows_to_render(self): - from bot_bottle.manifest import Manifest + from bot_bottle.manifest import ManifestIndex from bot_bottle.egress_addon_core import load_config, LOG_BLOCKS - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": { "log": 1, "routes": [{"host": "x.example"}], diff --git a/tests/unit/test_git_gate.py b/tests/unit/test_git_gate.py index 29ade69..939d531 100644 --- a/tests/unit/test_git_gate.py +++ b/tests/unit/test_git_gate.py @@ -15,7 +15,7 @@ from bot_bottle.git_gate import ( git_gate_render_hook, git_gate_upstreams_for_bottle, ) -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from tests.fixtures import fixture_minimal, fixture_with_git @@ -280,7 +280,7 @@ class TestPrepare(unittest.TestCase): self.assertEqual(0o600, os.stat(upstream.known_hosts_file).st_mode & 0o777) def test_prepare_skips_known_hosts_file_when_key_missing(self): - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": {"dev": {"git-gate": {"repos": { "foo": { "url": "ssh://git@github.com/didericis/foo.git", diff --git a/tests/unit/test_manifest_agent_git_user.py b/tests/unit/test_manifest_agent_git_user.py index 25d18f7..80d1afc 100644 --- a/tests/unit/test_manifest_agent_git_user.py +++ b/tests/unit/test_manifest_agent_git_user.py @@ -1,14 +1,14 @@ """Unit: agent-level git-gate.user overlay + provenance (PRD 0027, PRD 0047). An agent file may declare `git-gate.user` (name/email). At -`Manifest.bottle_for()` it overlays the referenced bottle's +`ManifestIndex.load_for_agent()` it overlays the referenced bottle's `git-gate.user` per-field, agent-wins-on-non-empty. `git-gate.repos` is rejected on agents. `Manifest.git_identity_summary()` reports the effective identity with per-field `(agent)`/`(bottle)` provenance. -The `from_json_obj` path drives `Agent.from_dict` + `bottle_for`; -a temp-dir case locks the md loader (the `_AGENT_KEYS` allow + the -`git-gate` threading into `agent_dict`).""" +The `from_json_obj` path drives `Agent.from_dict` + the overlay in +load_for_agent; a temp-dir case locks the md loader (the `_AGENT_KEYS` +allow + the `git-gate` threading into `agent_dict`).""" from __future__ import annotations @@ -19,7 +19,7 @@ import textwrap import unittest from pathlib import Path -from bot_bottle.manifest import ManifestError, Manifest +from bot_bottle.manifest import ManifestError, Manifest, ManifestIndex def _error_message(callable_, *args, **kwargs) -> str: # type: ignore @@ -32,13 +32,28 @@ def _error_message(callable_, *args, **kwargs) -> str: # type: ignore def _manifest(*, bottle_user=None, agent_git=None) -> Manifest: # type: ignore + """Build an index with one agent 'impl' and load it, returning a Manifest.""" bottle: dict = {} # type: ignore if bottle_user is not None: bottle = {"git-gate": {"user": bottle_user}} agent: dict = {"skills": [], "prompt": "", "bottle": "dev"} # type: ignore if agent_git is not None: agent["git-gate"] = agent_git - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ + "bottles": {"dev": bottle}, + "agents": {"impl": agent}, + }).load_for_agent("impl") + + +def _index(*, bottle_user=None, agent_git=None) -> ManifestIndex: + """Build an index with one agent 'impl' without loading it.""" + bottle: dict = {} # type: ignore + if bottle_user is not None: + bottle = {"git-gate": {"user": bottle_user}} + agent: dict = {"skills": [], "prompt": "", "bottle": "dev"} # type: ignore + if agent_git is not None: + agent["git-gate"] = agent_git + return ManifestIndex.from_json_obj({ "bottles": {"dev": bottle}, "agents": {"impl": agent}, }) @@ -47,7 +62,7 @@ def _manifest(*, bottle_user=None, agent_git=None) -> Manifest: # type: ignore class TestAgentGitUserOverlay(unittest.TestCase): def test_agent_supplies_both_fields(self): m = _manifest(agent_git={"user": {"name": "a", "email": "a@b"}}) - u = m.bottle_for("impl").git_user + u = m.bottle.git_user self.assertEqual("a", u.name) self.assertEqual("a@b", u.email) @@ -56,7 +71,7 @@ class TestAgentGitUserOverlay(unittest.TestCase): bottle_user={"name": "B", "email": "b@c"}, agent_git={"user": {"name": "a"}}, ) - u = m.bottle_for("impl").git_user + u = m.bottle.git_user self.assertEqual("a", u.name) # agent wins self.assertEqual("b@c", u.email) # bottle falls through @@ -65,34 +80,40 @@ class TestAgentGitUserOverlay(unittest.TestCase): bottle_user={"name": "B", "email": "b@c"}, agent_git={"user": {"email": "a@b"}}, ) - u = m.bottle_for("impl").git_user + u = m.bottle.git_user self.assertEqual("B", u.name) self.assertEqual("a@b", u.email) def test_agent_identity_with_bottle_declaring_none(self): - m = _manifest(agent_git={"user": {"name": "a", "email": "a@b"}}) - self.assertTrue(m.bottles["dev"].git_user.is_empty()) - self.assertFalse(m.bottle_for("impl").git_user.is_empty()) + idx = _index(agent_git={"user": {"name": "a", "email": "a@b"}}) + # Raw bottle has no git_user; loaded manifest has merged git_user from agent + self.assertTrue(idx.bottles["dev"].git_user.is_empty()) + m = idx.load_for_agent("impl") + self.assertFalse(m.bottle.git_user.is_empty()) def test_bottle_only_identity_preserved_when_agent_silent(self): m = _manifest(bottle_user={"name": "B", "email": "b@c"}) - u = m.bottle_for("impl").git_user + u = m.bottle.git_user self.assertEqual("B", u.name) self.assertEqual("b@c", u.email) - def test_bottle_for_returns_same_instance_when_no_overlay(self): - m = _manifest(bottle_user={"name": "B"}) - self.assertIs(m.bottles["dev"], m.bottle_for("impl")) + def test_no_overlay_uses_bottle_instance_directly(self): + idx = _index(bottle_user={"name": "B"}) + m = idx.load_for_agent("impl") + # Agent has no git_user — bottle instance should be the same object + self.assertIs(idx.bottles["dev"], m.bottle) - def test_bottle_for_returns_same_instance_when_overlay_is_noop(self): - m = _manifest( + def test_noop_overlay_uses_bottle_instance_directly(self): + idx = _index( bottle_user={"name": "B", "email": "b@c"}, agent_git={"user": {"name": "B", "email": "b@c"}}, ) - self.assertIs(m.bottles["dev"], m.bottle_for("impl")) + m = idx.load_for_agent("impl") + # Agent git_user == bottle git_user — no replace needed + self.assertEqual(idx.bottles["dev"].git_user, m.bottle.git_user) def test_other_bottle_fields_untouched_by_overlay(self): - m = Manifest.from_json_obj({ + idx = ManifestIndex.from_json_obj({ "bottles": {"dev": { "env": {"FOO": "bar"}, "supervise": True, @@ -103,7 +124,7 @@ class TestAgentGitUserOverlay(unittest.TestCase): "git-gate": {"user": {"name": "a"}}, }}, }) - b = m.bottle_for("impl") + b = idx.load_for_agent("impl").bottle self.assertEqual("a", b.git_user.name) self.assertEqual({"FOO": "bar"}, dict(b.env)) self.assertTrue(b.supervise) @@ -131,7 +152,7 @@ class TestGitIdentitySummary(unittest.TestCase): m = _manifest(agent_git={"user": {"name": "a", "email": "a@b"}}) self.assertEqual( "name=a (agent), email=a@b (agent)", - m.git_identity_summary("impl"), + m.git_identity_summary(), ) def test_mixed_provenance(self): @@ -141,19 +162,19 @@ class TestGitIdentitySummary(unittest.TestCase): ) self.assertEqual( "name=a (agent), email=b@c (bottle)", - m.git_identity_summary("impl"), + m.git_identity_summary(), ) def test_bottle_only(self): m = _manifest(bottle_user={"name": "B", "email": "b@c"}) self.assertEqual( "name=B (bottle), email=b@c (bottle)", - m.git_identity_summary("impl"), + m.git_identity_summary(), ) def test_none_when_unset_anywhere(self): m = _manifest() - self.assertIsNone(m.git_identity_summary("impl")) + self.assertIsNone(m.git_identity_summary()) _BOTTLE_DEV = """ @@ -217,13 +238,13 @@ 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)).load_for_agent("impl") - u = m.bottle_for("impl").git_user + m = ManifestIndex.resolve(str(self.home)).load_for_agent("impl") + u = m.bottle.git_user self.assertEqual("agent-name", u.name) self.assertEqual("bottle@example.com", u.email) self.assertEqual( "name=agent-name (agent), email=bottle@example.com (bottle)", - m.git_identity_summary("impl"), + m.git_identity_summary(), ) def test_md_agent_repos_fails_at_preflight(self): @@ -232,7 +253,7 @@ class TestAgentGitUserMdLoader(unittest.TestCase): self._write("bottles/dev.md", _BOTTLE_DEV) self._write("agents/impl.md", _AGENT_WITH_REPOS) from bot_bottle.manifest import ManifestError - names = Manifest.resolve(str(self.home)) + names = ManifestIndex.resolve(str(self.home)) self.assertIn("impl", names.all_agent_names) with self.assertRaises(ManifestError) as ctx: names.load_for_agent("impl") diff --git a/tests/unit/test_manifest_egress.py b/tests/unit/test_manifest_egress.py index ed83c84..7daa67a 100644 --- a/tests/unit/test_manifest_egress.py +++ b/tests/unit/test_manifest_egress.py @@ -9,18 +9,18 @@ partial `auth` is an error, auth omission means unauthenticated.""" import unittest -from bot_bottle.manifest import ManifestError, Manifest +from bot_bottle.manifest import ManifestError, ManifestIndex def _bottle(routes): # type: ignore - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"routes": routes}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }).bottles["dev"] def _provider_bottle(provider, routes): # type: ignore - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": { "dev": { "agent_provider": {"template": provider}, @@ -32,7 +32,7 @@ def _provider_bottle(provider, routes): # type: ignore def _provider_config_bottle(agent_provider): # type: ignore - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": {"dev": {"agent_provider": agent_provider}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }).bottles["dev"] @@ -433,7 +433,7 @@ class TestRouteValidation(unittest.TestCase): self.assertEqual((), b.egress.routes) def test_no_egress_block_means_empty(self): - b = Manifest.from_json_obj({ + b = ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }).bottles["dev"] @@ -443,7 +443,7 @@ class TestRouteValidation(unittest.TestCase): class TestConfigShape(unittest.TestCase): def test_unknown_egress_key_rejected(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"wat": []}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, @@ -454,14 +454,14 @@ class TestConfigShape(unittest.TestCase): self.assertEqual(0, b.egress.Log) def test_log_level_1_accepted(self): - b = Manifest.from_json_obj({ + b = ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"log": 1, "routes": []}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }).bottles["dev"] self.assertEqual(1, b.egress.Log) def test_log_level_2_accepted(self): - b = Manifest.from_json_obj({ + b = ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"log": 2, "routes": []}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }).bottles["dev"] @@ -469,7 +469,7 @@ class TestConfigShape(unittest.TestCase): def test_log_invalid_level_rejected(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"log": 3}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, @@ -477,7 +477,7 @@ class TestConfigShape(unittest.TestCase): def test_log_bool_rejected(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"log": True}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, @@ -485,7 +485,7 @@ class TestConfigShape(unittest.TestCase): def test_log_string_rejected(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {"egress": {"log": "full"}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, diff --git a/tests/unit/test_manifest_extends.py b/tests/unit/test_manifest_extends.py index 247ce26..b96ea8b 100644 --- a/tests/unit/test_manifest_extends.py +++ b/tests/unit/test_manifest_extends.py @@ -12,7 +12,7 @@ from __future__ import annotations import unittest -from bot_bottle.manifest import ManifestError, Manifest +from bot_bottle.manifest import ManifestError, ManifestIndex def _error_message(callable_, *args, **kwargs) -> str: # type: ignore @@ -28,7 +28,7 @@ def _build(**bottles) -> Manifest: # type: ignore """Build a manifest with the given bottles and one trivial agent referencing the first bottle (so the manifest is valid).""" first = next(iter(bottles)) - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": bottles, "agents": { "demo": {"skills": [], "prompt": "", "bottle": first}, diff --git a/tests/unit/test_manifest_git.py b/tests/unit/test_manifest_git.py index f8cccc6..8d1b855 100644 --- a/tests/unit/test_manifest_git.py +++ b/tests/unit/test_manifest_git.py @@ -2,7 +2,7 @@ import unittest -from bot_bottle.manifest import ManifestError, Manifest +from bot_bottle.manifest import ManifestError, ManifestIndex def _manifest(repos: dict) -> dict: # type: ignore @@ -14,7 +14,7 @@ def _manifest(repos: dict) -> dict: # type: ignore class TestGitEntryParsing(unittest.TestCase): def test_parses_minimal_entry(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "bot-bottle": { "url": "ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -30,7 +30,7 @@ class TestGitEntryParsing(unittest.TestCase): self.assertEqual("didericis/bot-bottle.git", e.UpstreamPath) def test_default_port_is_22(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/didericis/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -41,7 +41,7 @@ class TestGitEntryParsing(unittest.TestCase): self.assertEqual("github.com", e.UpstreamHost) def test_host_key_optional(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -50,7 +50,7 @@ class TestGitEntryParsing(unittest.TestCase): self.assertEqual("", m.bottles["dev"].git[0].KnownHostKey) def test_host_key_stored(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -60,7 +60,7 @@ class TestGitEntryParsing(unittest.TestCase): self.assertEqual("ssh-ed25519 AAAA", m.bottles["dev"].git[0].KnownHostKey) def test_repo_name_becomes_Name(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "my-repo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -70,19 +70,19 @@ class TestGitEntryParsing(unittest.TestCase): def test_missing_url_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": {"key": {"provider": "static", "path": "/dev/null"}}, })) def test_missing_key_block_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": {"url": "ssh://git@github.com/foo.git"}, })) def test_unknown_key_in_entry_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -92,7 +92,7 @@ class TestGitEntryParsing(unittest.TestCase): def test_non_ssh_url_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "https://github.com/didericis/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -101,7 +101,7 @@ class TestGitEntryParsing(unittest.TestCase): def test_scp_style_url_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "git@github.com:didericis/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -110,7 +110,7 @@ class TestGitEntryParsing(unittest.TestCase): def test_url_without_user_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -119,7 +119,7 @@ class TestGitEntryParsing(unittest.TestCase): def test_url_without_path_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com", "key": {"provider": "static", "path": "/dev/null"}, @@ -128,7 +128,7 @@ class TestGitEntryParsing(unittest.TestCase): def test_non_numeric_port_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com:notaport/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -136,7 +136,7 @@ class TestGitEntryParsing(unittest.TestCase): })) def test_ip_literal_upstream(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "bot-bottle": { "url": "ssh://git@100.78.141.42:30009/didericis/bot-bottle.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -152,7 +152,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_two_repos_different_hosts_both_parsed(self): # Repo names come from dict keys; two distinct keys always produce # two distinct entries (uniqueness is guaranteed at the YAML/dict level). - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {"git-gate": {"repos": { "foo": { "url": "ssh://git@a.example/x.git", @@ -170,7 +170,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_legacy_ssh_field_dies_with_hint(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": { "dev": { "ssh": [{ @@ -187,7 +187,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_name_with_single_quote_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "o'reilly": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -196,7 +196,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_name_with_space_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "my repo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -205,7 +205,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_name_with_semicolon_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo;bar": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -214,7 +214,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_name_with_dollar_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo$bar": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -222,7 +222,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): })) def test_valid_name_with_dots_and_hyphens_accepted(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "my.repo-name_1": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -233,7 +233,7 @@ class TestGitEntryCrossValidation(unittest.TestCase): def test_legacy_git_key_dies_with_hint(self): msg = "" try: - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {"git": {"remotes": {}}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) @@ -247,7 +247,7 @@ class TestStaticKey(unittest.TestCase): """git-gate.repos entries with key.provider = "static".""" def test_static_key_minimal(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "bot-bottle": { "url": "ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git", "key": {"provider": "static", "path": "/home/user/.ssh/id_ed25519"}, @@ -260,7 +260,7 @@ class TestStaticKey(unittest.TestCase): self.assertEqual("/home/user/.ssh/id_ed25519", e.IdentityFile) def test_static_key_sets_identity_file_at_parse_time(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null"}, @@ -270,7 +270,7 @@ class TestStaticKey(unittest.TestCase): def test_static_key_missing_path_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static"}, @@ -279,7 +279,7 @@ class TestStaticKey(unittest.TestCase): def test_static_key_unknown_field_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "static", "path": "/dev/null", "api_url": "x"}, @@ -291,7 +291,7 @@ class TestGiteaKey(unittest.TestCase): """git-gate.repos entries with key.provider = "gitea".""" def test_gitea_key_minimal(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "bot-bottle": { "url": "ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git", "key": { @@ -308,7 +308,7 @@ class TestGiteaKey(unittest.TestCase): self.assertEqual("", e.IdentityFile) def test_gitea_key_with_api_url(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "repo": { "url": "ssh://git@gitea.example.com/org/repo.git", "key": { @@ -321,7 +321,7 @@ class TestGiteaKey(unittest.TestCase): self.assertEqual("https://gitea.example.com", m.bottles["dev"].git[0].Key.api_url) def test_gitea_key_has_no_identity_file_at_parse_time(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/didericis/foo.git", "key": {"provider": "gitea", "forge_token_env": "T"}, @@ -331,7 +331,7 @@ class TestGiteaKey(unittest.TestCase): def test_gitea_key_missing_forge_token_env_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "gitea"}, @@ -340,7 +340,7 @@ class TestGiteaKey(unittest.TestCase): def test_gitea_key_unknown_field_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": { @@ -357,7 +357,7 @@ class TestKeyBlockValidation(unittest.TestCase): def test_missing_provider_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"path": "/dev/null"}, @@ -366,7 +366,7 @@ class TestKeyBlockValidation(unittest.TestCase): def test_unknown_provider_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": { "url": "ssh://git@github.com/foo.git", "key": {"provider": "github"}, @@ -375,14 +375,14 @@ class TestKeyBlockValidation(unittest.TestCase): def test_missing_key_block_dies(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest({ + ManifestIndex.from_json_obj(_manifest({ "foo": {"url": "ssh://git@github.com/foo.git"}, })) class TestEmptyGitGateField(unittest.TestCase): def test_no_git_gate_field_yields_empty_tuple(self): - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) @@ -390,13 +390,13 @@ class TestEmptyGitGateField(unittest.TestCase): def test_git_gate_object_type_required(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {"git-gate": "not-a-dict"}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) def test_empty_repos_yields_empty_tuple(self): - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {"git-gate": {"repos": {}}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) diff --git a/tests/unit/test_manifest_git_user.py b/tests/unit/test_manifest_git_user.py index fee0091..074a12d 100644 --- a/tests/unit/test_manifest_git_user.py +++ b/tests/unit/test_manifest_git_user.py @@ -2,7 +2,7 @@ import unittest -from bot_bottle.manifest import ManifestError, ManifestGitUser, Manifest +from bot_bottle.manifest import ManifestError, ManifestGitUser, ManifestIndex def _error_message(callable_, *args, **kwargs) -> str: # type: ignore @@ -23,7 +23,7 @@ def _manifest(git_user): # type: ignore class TestGitUserParsing(unittest.TestCase): def test_parses_both_fields(self): - m = Manifest.from_json_obj(_manifest({ + m = ManifestIndex.from_json_obj(_manifest({ "name": "Eric Bauerfeld", "email": "eric+claude@dideric.is", })) @@ -33,13 +33,13 @@ class TestGitUserParsing(unittest.TestCase): self.assertFalse(u.is_empty()) def test_name_only(self): - m = Manifest.from_json_obj(_manifest({"name": "Bot"})) + m = ManifestIndex.from_json_obj(_manifest({"name": "Bot"})) u = m.bottles["dev"].git_user self.assertEqual("Bot", u.name) self.assertEqual("", u.email) def test_email_only(self): - m = Manifest.from_json_obj(_manifest({"email": "bot@example.com"})) + m = ManifestIndex.from_json_obj(_manifest({"email": "bot@example.com"})) u = m.bottles["dev"].git_user self.assertEqual("", u.name) self.assertEqual("bot@example.com", u.email) @@ -47,7 +47,7 @@ class TestGitUserParsing(unittest.TestCase): def test_omitted_defaults_to_empty(self): # No git.user block at all → empty GitUser, is_empty True → # provisioner skips the `git config` step entirely. - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) @@ -59,13 +59,13 @@ class TestGitUserParsing(unittest.TestCase): # / half-finished edit; fail loudly rather than silently # no-op (the operator clearly meant to configure something). msg = _error_message( - Manifest.from_json_obj, _manifest({"name": "", "email": ""}), + ManifestIndex.from_json_obj, _manifest({"name": "", "email": ""}), ) self.assertIn("neither name nor email", msg) def test_unknown_key_dies(self): msg = _error_message( - Manifest.from_json_obj, + ManifestIndex.from_json_obj, _manifest({"name": "Bot", "username": "bot"}), ) self.assertIn("unknown key", msg) @@ -73,19 +73,19 @@ class TestGitUserParsing(unittest.TestCase): def test_non_string_name_dies(self): msg = _error_message( - Manifest.from_json_obj, _manifest({"name": 42}), + ManifestIndex.from_json_obj, _manifest({"name": 42}), ) self.assertIn("git-gate.user.name must be a string", msg) def test_non_string_email_dies(self): msg = _error_message( - Manifest.from_json_obj, _manifest({"email": ["x@y.z"]}), + ManifestIndex.from_json_obj, _manifest({"email": ["x@y.z"]}), ) self.assertIn("git-gate.user.email must be a string", msg) def test_legacy_top_level_git_user_dies(self): msg = _error_message( - Manifest.from_json_obj, + ManifestIndex.from_json_obj, { "bottles": {"dev": {"git_user": {"name": "Bot"}}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, diff --git a/tests/unit/test_manifest_md_load.py b/tests/unit/test_manifest_md_load.py index b2d0cdf..4f04aae 100644 --- a/tests/unit/test_manifest_md_load.py +++ b/tests/unit/test_manifest_md_load.py @@ -11,7 +11,7 @@ import textwrap import unittest from pathlib import Path -from bot_bottle.manifest import ManifestError, Manifest +from bot_bottle.manifest import ManifestError, Manifest, ManifestIndex def _write(p: Path, text: str) -> None: @@ -45,7 +45,7 @@ _AGENT_IMPL = """ class _ResolveCase(unittest.TestCase): - """Drives `Manifest.resolve(cwd)` against a temp $HOME and a + """Drives `ManifestIndex.resolve(cwd)` against a temp $HOME and a temp cwd. Subclasses lay down fixture files in setUp.""" def setUp(self) -> None: @@ -71,8 +71,8 @@ class _ResolveCase(unittest.TestCase): def cwd_cb(self) -> Path: return self.cwd_root / ".bot-bottle" - def resolve(self) -> Manifest: - return Manifest.resolve(str(self.cwd_root)) + def resolve(self) -> ManifestIndex: + return ManifestIndex.resolve(str(self.cwd_root)) class TestBottleFileParses(_ResolveCase): @@ -83,8 +83,7 @@ class TestBottleFileParses(_ResolveCase): _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) _write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL) m = self.resolve().load_for_agent("implementer") - self.assertIn("dev", m.bottles) - routes = m.bottles["dev"].egress.routes + routes = m.bottle.egress.routes self.assertEqual(2, len(routes)) self.assertEqual("api.anthropic.com", routes[0].Host) self.assertEqual("Bearer", routes[0].AuthScheme) @@ -101,7 +100,7 @@ class TestAgentFileParses(_ResolveCase): _write(self.home_cb / "bottles" / "dev.md", _BOTTLE_DEV) _write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL) m = self.resolve().load_for_agent("implementer") - a = m.agents["implementer"] + a = m.agent self.assertEqual("dev", a.bottle) self.assertEqual(("init-prd",), a.skills) # Body became the prompt; whitespace stripped. @@ -129,9 +128,9 @@ class TestCwdAgentOverridesHome(_ResolveCase): """, ) 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)) + self.assertIn("CWD-OVERRIDE-PROMPT", m.agent.prompt) + # Home bottle still present with its two egress routes + self.assertEqual(2, len(m.bottle.egress.routes)) class TestCwdBottlesIgnored(_ResolveCase): @@ -159,7 +158,7 @@ class TestCwdBottlesIgnored(_ResolveCase): # Home value wins because cwd bottles are ignored entirely. self.assertEqual( "api.anthropic.com", - m.bottles["dev"].egress.routes[0].Host, + m.bottle.egress.routes[0].Host, ) @@ -176,12 +175,12 @@ class TestStdlibOnly(unittest.TestCase): class TestExistingFromJsonObjStillWorks(unittest.TestCase): - """SC #6: `Manifest.from_json_obj` continues to work as a + """SC #6: `ManifestIndex.from_json_obj` continues to work as a programmatic entry point even though disk loading moved to the MD layout.""" def test_from_json_obj(self): - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "hi", "bottle": "dev"}}, @@ -216,8 +215,8 @@ class TestAgentFileDoublesAsClaudeCodeSubagent(_ResolveCase): """, ) m = self.resolve().load_for_agent("implementer") - self.assertEqual("dev", m.agents["implementer"].bottle) - self.assertEqual(("init-prd",), m.agents["implementer"].skills) + self.assertEqual("dev", m.agent.bottle) + self.assertEqual(("init-prd",), m.agent.skills) class TestManifestEntryPointParity(_ResolveCase): @@ -229,7 +228,7 @@ class TestManifestEntryPointParity(_ResolveCase): _write(self.home_cb / "agents" / "implementer.md", _AGENT_IMPL) md_manifest = self.resolve().load_for_agent("implementer") - json_manifest = Manifest.from_json_obj({ + json_index = ManifestIndex.from_json_obj({ "bottles": { "dev": { "egress": { @@ -256,17 +255,17 @@ class TestManifestEntryPointParity(_ResolveCase): }) self.assertEqual( - md_manifest.agents["implementer"], - json_manifest.agents["implementer"], + md_manifest.agent, + json_index.agents["implementer"], ) self.assertEqual( - md_manifest.bottles["dev"].egress.routes, - json_manifest.bottles["dev"].egress.routes, + md_manifest.bottle.egress.routes, + json_index.bottles["dev"].egress.routes, ) def test_json_agent_rejects_unknown_keys(self): with self.assertRaises(ManifestError): - Manifest.from_json_obj({ + ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": { "implementer": { @@ -277,7 +276,7 @@ class TestManifestEntryPointParity(_ResolveCase): }) def test_json_agent_accepts_claude_code_passthrough_keys(self): - manifest = Manifest.from_json_obj({ + index = ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": { "implementer": { @@ -291,7 +290,7 @@ class TestManifestEntryPointParity(_ResolveCase): }, }) - self.assertEqual("dev", manifest.agents["implementer"].bottle) + self.assertEqual("dev", index.agents["implementer"].bottle) class TestBrokenAgentOnlyFailsAtPreflight(_ResolveCase): @@ -359,7 +358,7 @@ class TestBrokenAgentOnlyFailsAtPreflight(_ResolveCase): self.assertIn("broken-agent", m.all_agent_names) # Valid agent loads fine. full = m.load_for_agent("implementer") - self.assertIn("implementer", full.agents) + self.assertEqual("dev", full.agent.bottle) # Broken bottle's agent raises at preflight. with self.assertRaises(ManifestError): m.load_for_agent("broken-agent") @@ -385,7 +384,7 @@ class TestNoManifestDies(_ResolveCase): self.resolve() def test_missing_ok_returns_empty_manifest(self): - m = Manifest.resolve(str(self.cwd_root), missing_ok=True) + m = ManifestIndex.resolve(str(self.cwd_root), missing_ok=True) self.assertEqual({}, dict(m.bottles)) self.assertEqual({}, dict(m.agents)) @@ -411,7 +410,7 @@ class TestUnknownBottleReferenceFailsAtPreflight(_ResolveCase): self.assertIn("implementer", m.all_agent_names) # Valid agent loads fine. full = m.load_for_agent("implementer") - self.assertIn("implementer", full.agents) + self.assertEqual("dev", full.agent.bottle) # Stray agent fails at preflight. with self.assertRaises(ManifestError): m.load_for_agent("stray") @@ -431,7 +430,3 @@ class TestFilenameValidation(_ResolveCase): self.assertIn("implementer", m.all_agent_names) self.assertNotIn("BadName", m.all_agent_names) self.assertNotIn("badname", m.all_agent_names) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/unit/test_manifest_runtime.py b/tests/unit/test_manifest_runtime.py index 42def91..6015324 100644 --- a/tests/unit/test_manifest_runtime.py +++ b/tests/unit/test_manifest_runtime.py @@ -7,7 +7,7 @@ silently ignoring.""" import unittest from typing import Any -from bot_bottle.manifest import ManifestError, ManifestBottle, Manifest +from bot_bottle.manifest import ManifestError, ManifestBottle, ManifestIndex def _manifest_with_runtime(value: object) -> dict[str, Any]: @@ -19,7 +19,7 @@ def _manifest_with_runtime(value: object) -> dict[str, Any]: class TestManifestRuntimeRemoved(unittest.TestCase): def test_loads_when_runtime_absent(self): - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, }) @@ -32,7 +32,7 @@ class TestManifestRuntimeRemoved(unittest.TestCase): for value in ("runsc", "runc", "kata-runtime", "", 42, None): with self.subTest(value=value): with self.assertRaises(ManifestError): - Manifest.from_json_obj(_manifest_with_runtime(value)) + ManifestIndex.from_json_obj(_manifest_with_runtime(value)) if __name__ == "__main__": diff --git a/tests/unit/test_plan_print_parity.py b/tests/unit/test_plan_print_parity.py index 3001fc3..f80b079 100644 --- a/tests/unit/test_plan_print_parity.py +++ b/tests/unit/test_plan_print_parity.py @@ -19,14 +19,14 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.backend.smolmachines.bottle_plan import SmolmachinesBottlePlan from bot_bottle.egress import EgressPlan, EgressRoute from bot_bottle.git_gate import GitGatePlan, GitGateUpstream -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import Manifest, ManifestIndex def _manifest() -> Manifest: - return Manifest.from_json_obj({ + return ManifestIndex.from_json_obj({ "bottles": {"dev": {}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }) + }).load_for_agent("demo") def _spec(manifest: Manifest, tmp: str) -> BottleSpec: diff --git a/tests/unit/test_provision_git.py b/tests/unit/test_provision_git.py index e8eb549..6023333 100644 --- a/tests/unit/test_provision_git.py +++ b/tests/unit/test_provision_git.py @@ -10,7 +10,7 @@ from bot_bottle.git_gate import ( GIT_GATE_HOSTNAME, git_gate_render_gitconfig, ) -from bot_bottle.manifest import Manifest +from bot_bottle.manifest import ManifestIndex from tests.fixtures import fixture_minimal, fixture_with_git @@ -72,7 +72,7 @@ class TestGitGateGitconfigRender(unittest.TestCase): def test_ip_upstream_emits_single_insteadof(self): # In the new format the dict key is the repo name, not a host # alias, so there is only one insteadOf rule — for the IP URL. - m = Manifest.from_json_obj({ + m = ManifestIndex.from_json_obj({ "bottles": {"dev": {"git-gate": {"repos": { "bot-bottle": { "url": "ssh://git@100.78.141.42:30009/didericis/bot-bottle.git", diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index 3424c6b..3bbfe8c 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -33,7 +33,7 @@ from bot_bottle.backend.smolmachines.launch import _bundle_launch_spec from bot_bottle.backend.util import AGENT_CA_PATH from bot_bottle.egress import EgressPlan, EgressRoute from bot_bottle.git_gate import GitGatePlan, GitGateUpstream -from bot_bottle.manifest import ManifestGitEntry, ManifestKeyConfig, Manifest +from bot_bottle.manifest import ManifestGitEntry, ManifestKeyConfig, Manifest, ManifestIndex from bot_bottle.supervise import SupervisePlan @@ -110,7 +110,7 @@ def _plan( bottle_json["git-gate"] = git_gate_json if supervise: bottle_json["supervise"] = True - manifest = Manifest.from_json_obj({ + manifest = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": { "demo": { @@ -119,7 +119,7 @@ def _plan( "bottle": "dev", }, }, - }) + }).load_for_agent("demo") spec = BottleSpec( manifest=manifest, agent_name="demo", -- 2.52.0 From 56ef71060ae2a4e43c8cfa738d638ddeb5bf80a3 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 23 Jun 2026 02:08:27 +0000 Subject: [PATCH 7/8] fix(types): add BottleSpec.loaded_manifest to satisfy pyright on union type BottleSpec.manifest is ManifestIndex | Manifest (pre/post _validate()). Downstream code always runs post-validate so it needs Manifest, but pyright flagged every .agent/.bottle access. The new loaded_manifest property asserts isinstance and returns Manifest, giving pyright a narrowed type without scattering type: ignore everywhere. Also remove unused Manifest imports from test files and annotate the _index() helper in test_manifest_agent_git_user. --- bot_bottle/agent_provider.py | 2 +- bot_bottle/backend/__init__.py | 18 ++++++++++++++---- bot_bottle/backend/docker/launch.py | 2 +- bot_bottle/backend/macos_container/launch.py | 2 +- bot_bottle/backend/resolve_common.py | 3 +-- bot_bottle/backend/smolmachines/launch.py | 2 +- bot_bottle/contrib/claude/agent_provider.py | 4 ++-- bot_bottle/contrib/codex/agent_provider.py | 4 ++-- bot_bottle/contrib/pi/agent_provider.py | 2 +- tests/unit/test_contrib_claude_provider.py | 2 +- tests/unit/test_contrib_codex_provider.py | 2 +- tests/unit/test_contrib_pi_provider.py | 2 +- tests/unit/test_docker_provision_git_user.py | 2 +- tests/unit/test_manifest_agent_git_user.py | 2 +- tests/unit/test_manifest_md_load.py | 2 +- tests/unit/test_smolmachines_provision.py | 2 +- 16 files changed, 31 insertions(+), 22 deletions(-) diff --git a/bot_bottle/agent_provider.py b/bot_bottle/agent_provider.py index 20f0bb7..da03221 100644 --- a/bot_bottle/agent_provider.py +++ b/bot_bottle/agent_provider.py @@ -240,7 +240,7 @@ class AgentProvider(ABC): BottleBackend.provision_workspace against the running bottle.""" from .log import info - manifest_bottle = plan.spec.manifest.bottle + manifest_bottle = plan.spec.loaded_manifest.bottle if manifest_bottle.git: from .git_gate import GIT_GATE_HOSTNAME, git_gate_render_gitconfig gate_host = getattr(plan, "git_gate_insteadof_host", GIT_GATE_HOSTNAME) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index ad32351..6beffc0 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -65,6 +65,13 @@ class BottleSpec: agent_name: str copy_cwd: bool user_cwd: str + + @property + def loaded_manifest(self) -> Manifest: + assert isinstance(self.manifest, Manifest), ( + "spec.manifest is still a ManifestIndex — call _validate() first" + ) + return self.manifest # PRD 0016 follow-up: when set, the backend's prepare step uses # this identity instead of minting a fresh one — the resume path # (`cli.py resume `) sets this to continue an existing @@ -112,7 +119,7 @@ class BottlePlan(ABC): """Render the y/N preflight summary to stderr.""" del remote_control spec = self.spec - manifest = spec.manifest # type: ignore[assignment] + manifest = spec.loaded_manifest agent = manifest.agent bottle = manifest.bottle @@ -293,7 +300,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): self._preflight() - manifest = spec.manifest # type: ignore[assignment] + manifest = spec.loaded_manifest manifest_bottle = manifest.bottle manifest_agent_provider = manifest_bottle.agent_provider agent_provider = get_provider(manifest_agent_provider.template) @@ -364,7 +371,10 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): 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) # type: ignore[union-attr] + assert isinstance(spec.manifest, ManifestIndex), ( + "_validate() called on a spec whose manifest is already loaded" + ) + manifest = spec.manifest.load_for_agent(spec.agent_name) spec = replace(spec, manifest=manifest) agent = manifest.agent self._validate_skills(agent.skills) @@ -384,7 +394,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): ) def _validate_agent_provider_dockerfile(self, spec: BottleSpec) -> None: - manifest = spec.manifest # type: ignore[assignment] + manifest = spec.loaded_manifest bottle = manifest.bottle dockerfile = bottle.agent_provider.dockerfile if not dockerfile: diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index f937b30..e0f7f7c 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -75,7 +75,7 @@ def launch( Teardown on exit.""" stack = ExitStack() - _bottle_for_revoke = plan.spec.manifest.bottle + _bottle_for_revoke = plan.spec.loaded_manifest.bottle _git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) def teardown() -> None: diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 9db7411..a0e9d26 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -68,7 +68,7 @@ def launch( ) -> Generator[MacosContainerBottle, None, None]: """Build, run, provision, and yield an Apple Container bottle.""" stack = ExitStack() - bottle_for_revoke = plan.spec.manifest.bottle + bottle_for_revoke = plan.spec.loaded_manifest.bottle git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) def teardown() -> None: diff --git a/bot_bottle/backend/resolve_common.py b/bot_bottle/backend/resolve_common.py index affb345..21b483d 100644 --- a/bot_bottle/backend/resolve_common.py +++ b/bot_bottle/backend/resolve_common.py @@ -69,8 +69,7 @@ def write_launch_metadata( def prepare_agent_state_dir(slug: str, spec: BottleSpec) -> tuple[Path, Path]: """Create the agent state subdir, write the prompt file. Returns (agent_dir, prompt_file).""" - manifest = spec.manifest # type: ignore[assignment] - agent = manifest.agent + agent = spec.loaded_manifest.agent agent_dir = agent_state_dir(slug) agent_dir.mkdir(parents=True, exist_ok=True) prompt_file = agent_dir / "prompt.txt" diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index e16b611..93a4d50 100644 --- a/bot_bottle/backend/smolmachines/launch.py +++ b/bot_bottle/backend/smolmachines/launch.py @@ -130,7 +130,7 @@ def _teardown_smolmachines( except BaseException as exc: # noqa: W0718 — teardown must not fail teardown_exc = exc warn(f"smolmachines teardown failed: {exc!r}") - bottle = plan.spec.manifest.bottle + bottle = plan.spec.loaded_manifest.bottle revoke_git_gate_provisioned_keys(bottle, git_gate_state_dir(plan.slug)) if teardown_exc is not None: raise teardown_exc diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index add66a2..91733cd 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -211,7 +211,7 @@ class ClaudeAgentProvider(AgentProvider): when the agent has no skills.""" from ...backend.util import host_skill_dir - agent = plan.spec.manifest.agent + agent = plan.spec.loaded_manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) @@ -240,7 +240,7 @@ class ClaudeAgentProvider(AgentProvider): f"chown node:node {prompt_path} && chmod 600 {prompt_path}", user="root", ) - agent = plan.spec.manifest.agent + agent = plan.spec.loaded_manifest.agent return prompt_path if plan.agent_provision.has_prompt or agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 8b5f485..997a30f 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -177,7 +177,7 @@ class CodexAgentProvider(AgentProvider): skills.""" from ...backend.util import host_skill_dir - agent = plan.spec.manifest.agent + agent = plan.spec.loaded_manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) @@ -206,7 +206,7 @@ class CodexAgentProvider(AgentProvider): f"chown node:node {prompt_path} && chmod 600 {prompt_path}", user="root", ) - agent = plan.spec.manifest.agent + agent = plan.spec.loaded_manifest.agent return prompt_path if plan.agent_provision.has_prompt or agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: diff --git a/bot_bottle/contrib/pi/agent_provider.py b/bot_bottle/contrib/pi/agent_provider.py index 83d498d..73b12c3 100644 --- a/bot_bottle/contrib/pi/agent_provider.py +++ b/bot_bottle/contrib/pi/agent_provider.py @@ -232,7 +232,7 @@ class PiAgentProvider(AgentProvider): def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None: from ...backend.util import host_skill_dir - agent = plan.spec.manifest.agent + agent = plan.spec.loaded_manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) diff --git a/tests/unit/test_contrib_claude_provider.py b/tests/unit/test_contrib_claude_provider.py index c8ce4c0..cb806e8 100644 --- a/tests/unit/test_contrib_claude_provider.py +++ b/tests/unit/test_contrib_claude_provider.py @@ -24,7 +24,7 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.contrib.claude.agent_provider import ClaudeAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest, ManifestIndex +from bot_bottle.manifest import ManifestIndex from bot_bottle.supervise import SupervisePlan diff --git a/tests/unit/test_contrib_codex_provider.py b/tests/unit/test_contrib_codex_provider.py index 4e5411c..8ba5b97 100644 --- a/tests/unit/test_contrib_codex_provider.py +++ b/tests/unit/test_contrib_codex_provider.py @@ -24,7 +24,7 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.contrib.codex.agent_provider import CodexAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest, ManifestIndex +from bot_bottle.manifest import ManifestIndex from bot_bottle.supervise import SupervisePlan diff --git a/tests/unit/test_contrib_pi_provider.py b/tests/unit/test_contrib_pi_provider.py index dd0a88c..f9d83c2 100644 --- a/tests/unit/test_contrib_pi_provider.py +++ b/tests/unit/test_contrib_pi_provider.py @@ -16,7 +16,7 @@ from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.contrib.pi.agent_provider import PiAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest, ManifestIndex +from bot_bottle.manifest import ManifestIndex _URL = "http://supervise:9100/" diff --git a/tests/unit/test_docker_provision_git_user.py b/tests/unit/test_docker_provision_git_user.py index 6c1220c..4c572df 100644 --- a/tests/unit/test_docker_provision_git_user.py +++ b/tests/unit/test_docker_provision_git_user.py @@ -21,7 +21,7 @@ from bot_bottle.backend import Bottle, BottleSpec, ExecResult from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest, ManifestIndex +from bot_bottle.manifest import ManifestIndex class _Provider(AgentProvider): diff --git a/tests/unit/test_manifest_agent_git_user.py b/tests/unit/test_manifest_agent_git_user.py index 80d1afc..c9ccd26 100644 --- a/tests/unit/test_manifest_agent_git_user.py +++ b/tests/unit/test_manifest_agent_git_user.py @@ -45,7 +45,7 @@ def _manifest(*, bottle_user=None, agent_git=None) -> Manifest: # type: ignore }).load_for_agent("impl") -def _index(*, bottle_user=None, agent_git=None) -> ManifestIndex: +def _index(*, bottle_user: dict[str, object] | None = None, agent_git: dict[str, object] | None = None) -> ManifestIndex: """Build an index with one agent 'impl' without loading it.""" bottle: dict = {} # type: ignore if bottle_user is not None: diff --git a/tests/unit/test_manifest_md_load.py b/tests/unit/test_manifest_md_load.py index 4f04aae..7659469 100644 --- a/tests/unit/test_manifest_md_load.py +++ b/tests/unit/test_manifest_md_load.py @@ -11,7 +11,7 @@ import textwrap import unittest from pathlib import Path -from bot_bottle.manifest import ManifestError, Manifest, ManifestIndex +from bot_bottle.manifest import ManifestError, ManifestIndex def _write(p: Path, text: str) -> None: diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index 3bbfe8c..5864916 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -33,7 +33,7 @@ from bot_bottle.backend.smolmachines.launch import _bundle_launch_spec from bot_bottle.backend.util import AGENT_CA_PATH from bot_bottle.egress import EgressPlan, EgressRoute from bot_bottle.git_gate import GitGatePlan, GitGateUpstream -from bot_bottle.manifest import ManifestGitEntry, ManifestKeyConfig, Manifest, ManifestIndex +from bot_bottle.manifest import ManifestGitEntry, ManifestKeyConfig, ManifestIndex from bot_bottle.supervise import SupervisePlan -- 2.52.0 From da42740156adfea10889ca4cebc89ef986236033 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 23 Jun 2026 02:22:10 +0000 Subject: [PATCH 8/8] refactor(types): move loaded manifest from BottleSpec to BottlePlan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BottleSpec.manifest was ManifestIndex | Manifest — a union encoding two lifecycle stages in one field. The union was unjustifiable: it forced a type-narrowing workaround (loaded_manifest property) on every consumer. Clean split: - BottleSpec.manifest: ManifestIndex (always; CLI-supplied intent) - BottlePlan.manifest: Manifest (always; loaded by _validate()) _validate() returns the loaded Manifest directly. prepare() passes it to _resolve_plan(), which stores it on the plan. All provisioner code now reads plan.manifest.agent / plan.manifest.bottle — no union, no asserts, no type: ignore. --- bot_bottle/agent_provider.py | 2 +- bot_bottle/backend/__init__.py | 44 +++++++------------ bot_bottle/backend/docker/backend.py | 3 ++ bot_bottle/backend/docker/launch.py | 2 +- bot_bottle/backend/docker/resolve_plan.py | 3 ++ bot_bottle/backend/macos_container/backend.py | 3 ++ bot_bottle/backend/macos_container/launch.py | 2 +- .../backend/macos_container/resolve_plan.py | 3 ++ bot_bottle/backend/resolve_common.py | 6 +-- bot_bottle/backend/smolmachines/backend.py | 3 ++ bot_bottle/backend/smolmachines/launch.py | 2 +- .../backend/smolmachines/resolve_plan.py | 3 ++ bot_bottle/contrib/claude/agent_provider.py | 4 +- bot_bottle/contrib/codex/agent_provider.py | 4 +- bot_bottle/contrib/pi/agent_provider.py | 2 +- tests/unit/test_compose.py | 19 ++++---- tests/unit/test_contrib_claude_provider.py | 8 ++-- tests/unit/test_contrib_codex_provider.py | 8 ++-- tests/unit/test_contrib_pi_provider.py | 8 ++-- tests/unit/test_docker_launch_teardown.py | 18 +++----- tests/unit/test_docker_provision_git_user.py | 8 ++-- tests/unit/test_macos_container_launch.py | 8 ++++ tests/unit/test_plan_print_parity.py | 35 ++++++++------- tests/unit/test_smolmachines_provision.py | 8 ++-- 24 files changed, 112 insertions(+), 94 deletions(-) diff --git a/bot_bottle/agent_provider.py b/bot_bottle/agent_provider.py index da03221..ca2a6ab 100644 --- a/bot_bottle/agent_provider.py +++ b/bot_bottle/agent_provider.py @@ -240,7 +240,7 @@ class AgentProvider(ABC): BottleBackend.provision_workspace against the running bottle.""" from .log import info - manifest_bottle = plan.spec.loaded_manifest.bottle + manifest_bottle = plan.manifest.bottle if manifest_bottle.git: from .git_gate import GIT_GATE_HOSTNAME, git_gate_render_gitconfig gate_host = getattr(plan, "git_gate_insteadof_host", GIT_GATE_HOSTNAME) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 6beffc0..36bbb37 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, replace +from dataclasses import dataclass from pathlib import Path from typing import Any, Generic, Sequence, TypeVar @@ -61,17 +61,10 @@ class BottleSpec: Resolved values (image names, container name, scratch paths, runsc availability) live on the plan, not the spec.""" - manifest: ManifestIndex | Manifest + manifest: ManifestIndex agent_name: str copy_cwd: bool user_cwd: str - - @property - def loaded_manifest(self) -> Manifest: - assert isinstance(self.manifest, Manifest), ( - "spec.manifest is still a ManifestIndex — call _validate() first" - ) - return self.manifest # PRD 0016 follow-up: when set, the backend's prepare step uses # this identity instead of minting a fresh one — the resume path # (`cli.py resume `) sets this to continue an existing @@ -87,6 +80,7 @@ class BottlePlan(ABC): (e.g. DockerBottlePlan) add backend-specific resolved fields.""" spec: BottleSpec + manifest: Manifest stage_dir: Path git_gate_plan: GitGatePlan @@ -119,7 +113,7 @@ class BottlePlan(ABC): """Render the y/N preflight summary to stderr.""" del remote_control spec = self.spec - manifest = spec.loaded_manifest + manifest = self.manifest agent = manifest.agent bottle = manifest.bottle @@ -296,11 +290,10 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): write_launch_metadata, ) - spec = self._validate(spec) + manifest = self._validate(spec) self._preflight() - manifest = spec.loaded_manifest manifest_bottle = manifest.bottle manifest_agent_provider = manifest_bottle.agent_provider agent_provider = get_provider(manifest_agent_provider.template) @@ -320,7 +313,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): else: agent_dockerfile_path = str(agent_provider.dockerfile) - agent_dir, prompt_file = prepare_agent_state_dir(slug, spec) + agent_dir, prompt_file = prepare_agent_state_dir(slug, manifest) agent_provision_plan = build_agent_provision_plan( template=manifest_agent_provider.template, @@ -344,6 +337,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): return self._resolve_plan( spec, + manifest=manifest, slug=slug, resolved_env=resolved_env, agent_provision_plan=agent_provision_plan, @@ -362,24 +356,18 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): """ pass - def _validate(self, spec: BottleSpec) -> BottleSpec: + def _validate(self, spec: BottleSpec) -> Manifest: """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.""" - assert isinstance(spec.manifest, ManifestIndex), ( - "_validate() called on a spec whose manifest is already loaded" - ) + Returns the loaded Manifest for the selected agent. Subclasses with + additional preconditions should override and call + `super()._validate(spec)` first.""" manifest = spec.manifest.load_for_agent(spec.agent_name) - spec = replace(spec, manifest=manifest) - agent = manifest.agent - self._validate_skills(agent.skills) - self._validate_agent_provider_dockerfile(spec) - return spec + self._validate_skills(manifest.agent.skills) + self._validate_agent_provider_dockerfile(spec, manifest) + return manifest def _validate_skills(self, skills: Sequence[str]) -> None: """Each named skill must be a directory under the host's @@ -393,8 +381,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): f"Create it under ~/.claude/skills/, then re-run." ) - def _validate_agent_provider_dockerfile(self, spec: BottleSpec) -> None: - manifest = spec.loaded_manifest + def _validate_agent_provider_dockerfile(self, spec: BottleSpec, manifest: Manifest) -> None: bottle = manifest.bottle dockerfile = bottle.agent_provider.dockerfile if not dockerfile: @@ -412,6 +399,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): def _resolve_plan(self, spec: BottleSpec, *, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, diff --git a/bot_bottle/backend/docker/backend.py b/bot_bottle/backend/docker/backend.py index 23c7d1e..b3b9e3f 100644 --- a/bot_bottle/backend/docker/backend.py +++ b/bot_bottle/backend/docker/backend.py @@ -30,6 +30,7 @@ from ...egress import EgressPlan from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan +from ...manifest import Manifest from .. import ActiveAgent, BottleBackend, BottleSpec from . import cleanup as _cleanup from . import enumerate as _enumerate @@ -63,6 +64,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup self, spec: BottleSpec, *, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, @@ -73,6 +75,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup ) -> DockerBottlePlan: return _resolve_plan.resolve_plan( spec, + manifest=manifest, slug=slug, resolved_env=resolved_env, agent_provision_plan=agent_provision_plan, diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index e0f7f7c..c930b36 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -75,7 +75,7 @@ def launch( Teardown on exit.""" stack = ExitStack() - _bottle_for_revoke = plan.spec.loaded_manifest.bottle + _bottle_for_revoke = plan.manifest.bottle _git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) def teardown() -> None: diff --git a/bot_bottle/backend/docker/resolve_plan.py b/bot_bottle/backend/docker/resolve_plan.py index f07ecec..c48e3ac 100644 --- a/bot_bottle/backend/docker/resolve_plan.py +++ b/bot_bottle/backend/docker/resolve_plan.py @@ -18,6 +18,7 @@ from .. import BottleSpec from ...env import ResolvedEnv from ...agent_provider import AgentProvisionPlan from ...egress import EgressPlan +from ...manifest import Manifest from ...supervise import SupervisePlan from ...git_gate import GitGatePlan @@ -31,6 +32,7 @@ def build_guest_env(resolved_env: ResolvedEnv) -> dict[str, str]: def resolve_plan( spec: BottleSpec, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, @@ -48,6 +50,7 @@ def resolve_plan( return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage_dir, slug=slug, forwarded_env=dict(resolved_env.forwarded), diff --git a/bot_bottle/backend/macos_container/backend.py b/bot_bottle/backend/macos_container/backend.py index 14a0496..bd43ed8 100644 --- a/bot_bottle/backend/macos_container/backend.py +++ b/bot_bottle/backend/macos_container/backend.py @@ -11,6 +11,7 @@ from ...egress import EgressPlan from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan +from ...manifest import Manifest from .. import ActiveAgent, BottleBackend, BottleSpec from . import cleanup as _cleanup from . import enumerate as _enumerate @@ -45,6 +46,7 @@ class MacosContainerBottleBackend( self, spec: BottleSpec, *, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, @@ -55,6 +57,7 @@ class MacosContainerBottleBackend( ) -> MacosContainerBottlePlan: return _resolve_plan.resolve_plan( spec, + manifest=manifest, slug=slug, resolved_env=resolved_env, agent_provision_plan=agent_provision_plan, diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index a0e9d26..7f53256 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -68,7 +68,7 @@ def launch( ) -> Generator[MacosContainerBottle, None, None]: """Build, run, provision, and yield an Apple Container bottle.""" stack = ExitStack() - bottle_for_revoke = plan.spec.loaded_manifest.bottle + bottle_for_revoke = plan.manifest.bottle git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) def teardown() -> None: diff --git a/bot_bottle/backend/macos_container/resolve_plan.py b/bot_bottle/backend/macos_container/resolve_plan.py index 021571b..9a9eb28 100644 --- a/bot_bottle/backend/macos_container/resolve_plan.py +++ b/bot_bottle/backend/macos_container/resolve_plan.py @@ -9,6 +9,7 @@ from ...egress import EgressPlan from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan +from ...manifest import Manifest from .. import BottleSpec from . import util as container_mod from .bottle_plan import MacosContainerBottlePlan @@ -24,6 +25,7 @@ def build_guest_env(resolved_env: ResolvedEnv) -> dict[str, str]: def resolve_plan( spec: BottleSpec, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, @@ -34,6 +36,7 @@ def resolve_plan( ) -> MacosContainerBottlePlan: return MacosContainerBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage_dir, slug=slug, forwarded_env=dict(resolved_env.forwarded), diff --git a/bot_bottle/backend/resolve_common.py b/bot_bottle/backend/resolve_common.py index 21b483d..cb9322f 100644 --- a/bot_bottle/backend/resolve_common.py +++ b/bot_bottle/backend/resolve_common.py @@ -26,7 +26,7 @@ from ..bottle_state import ( ) from ..egress import Egress, EgressPlan from ..git_gate import GitGate, GitGatePlan -from ..manifest import ManifestBottle +from ..manifest import Manifest, ManifestBottle from ..supervise import Supervise, SupervisePlan from . import BottleSpec @@ -66,10 +66,10 @@ def write_launch_metadata( )) -def prepare_agent_state_dir(slug: str, spec: BottleSpec) -> tuple[Path, Path]: +def prepare_agent_state_dir(slug: str, manifest: Manifest) -> tuple[Path, Path]: """Create the agent state subdir, write the prompt file. Returns (agent_dir, prompt_file).""" - agent = spec.loaded_manifest.agent + agent = manifest.agent agent_dir = agent_state_dir(slug) agent_dir.mkdir(parents=True, exist_ok=True) prompt_file = agent_dir / "prompt.txt" diff --git a/bot_bottle/backend/smolmachines/backend.py b/bot_bottle/backend/smolmachines/backend.py index 4cd7fef..f24be61 100644 --- a/bot_bottle/backend/smolmachines/backend.py +++ b/bot_bottle/backend/smolmachines/backend.py @@ -18,6 +18,7 @@ from ...egress import EgressPlan from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan +from ...manifest import Manifest from .. import ActiveAgent, BottleBackend, BottleSpec from . import cleanup as _cleanup from . import enumerate as _enumerate @@ -55,6 +56,7 @@ class SmolmachinesBottleBackend( self, spec: BottleSpec, *, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, @@ -65,6 +67,7 @@ class SmolmachinesBottleBackend( ) -> SmolmachinesBottlePlan: return _resolve_plan.resolve_plan( spec, + manifest=manifest, slug=slug, resolved_env=resolved_env, agent_provision_plan=agent_provision_plan, diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index 93a4d50..7bf891a 100644 --- a/bot_bottle/backend/smolmachines/launch.py +++ b/bot_bottle/backend/smolmachines/launch.py @@ -130,7 +130,7 @@ def _teardown_smolmachines( except BaseException as exc: # noqa: W0718 — teardown must not fail teardown_exc = exc warn(f"smolmachines teardown failed: {exc!r}") - bottle = plan.spec.loaded_manifest.bottle + bottle = plan.manifest.bottle revoke_git_gate_provisioned_keys(bottle, git_gate_state_dir(plan.slug)) if teardown_exc is not None: raise teardown_exc diff --git a/bot_bottle/backend/smolmachines/resolve_plan.py b/bot_bottle/backend/smolmachines/resolve_plan.py index e49eece..8d8dfcb 100644 --- a/bot_bottle/backend/smolmachines/resolve_plan.py +++ b/bot_bottle/backend/smolmachines/resolve_plan.py @@ -13,6 +13,7 @@ from __future__ import annotations from pathlib import Path from .. import BottleSpec +from ...manifest import Manifest from ...env import ResolvedEnv from ...agent_provider import AgentProvisionPlan from ...egress import EgressPlan @@ -46,6 +47,7 @@ def build_guest_env(resolved_env: ResolvedEnv) -> dict[str, str]: def resolve_plan( spec: BottleSpec, + manifest: Manifest, slug: str, resolved_env: ResolvedEnv, agent_provision_plan: AgentProvisionPlan, @@ -67,6 +69,7 @@ def resolve_plan( return SmolmachinesBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage_dir, slug=slug, bundle_subnet=subnet, diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index 91733cd..198d1dd 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -211,7 +211,7 @@ class ClaudeAgentProvider(AgentProvider): when the agent has no skills.""" from ...backend.util import host_skill_dir - agent = plan.spec.loaded_manifest.agent + agent = plan.manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) @@ -240,7 +240,7 @@ class ClaudeAgentProvider(AgentProvider): f"chown node:node {prompt_path} && chmod 600 {prompt_path}", user="root", ) - agent = plan.spec.loaded_manifest.agent + agent = plan.manifest.agent return prompt_path if plan.agent_provision.has_prompt or agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 997a30f..386c838 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -177,7 +177,7 @@ class CodexAgentProvider(AgentProvider): skills.""" from ...backend.util import host_skill_dir - agent = plan.spec.loaded_manifest.agent + agent = plan.manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) @@ -206,7 +206,7 @@ class CodexAgentProvider(AgentProvider): f"chown node:node {prompt_path} && chmod 600 {prompt_path}", user="root", ) - agent = plan.spec.loaded_manifest.agent + agent = plan.manifest.agent return prompt_path if plan.agent_provision.has_prompt or agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: diff --git a/bot_bottle/contrib/pi/agent_provider.py b/bot_bottle/contrib/pi/agent_provider.py index 73b12c3..dd88913 100644 --- a/bot_bottle/contrib/pi/agent_provider.py +++ b/bot_bottle/contrib/pi/agent_provider.py @@ -232,7 +232,7 @@ class PiAgentProvider(AgentProvider): def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None: from ...backend.util import host_skill_dir - agent = plan.spec.loaded_manifest.agent + agent = plan.manifest.agent if not agent.skills: return skills_dir = _skills_dir(plan.guest_home) diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index dafe3ef..cad8840 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -67,16 +67,6 @@ def _manifest(*, supervise: bool, with_git: bool, with_egress: bool) -> Manifest }) -def _spec(*, supervise: bool, with_git: bool, with_egress: bool) -> BottleSpec: - return BottleSpec( - manifest=_manifest( - supervise=supervise, with_git=with_git, with_egress=with_egress, - ), - agent_name="demo", - copy_cwd=False, - user_cwd="/tmp/x", - ) - def _git_gate_plan(upstreams: tuple[GitGateUpstream, ...] = ()) -> GitGatePlan: return GitGatePlan( @@ -146,9 +136,16 @@ def _plan( roles=(), ),) - spec = _spec(supervise=supervise, with_git=with_git, with_egress=with_egress) + index = _manifest(supervise=supervise, with_git=with_git, with_egress=with_egress) + spec = BottleSpec( + manifest=index, + agent_name="demo", + copy_cwd=False, + user_cwd="/tmp/x", + ) return DockerBottlePlan( spec=spec, + manifest=index.load_for_agent("demo"), stage_dir=STAGE, slug=SLUG, forwarded_env={"CLAUDE_CODE_OAUTH_TOKEN": "x"}, diff --git a/tests/unit/test_contrib_claude_provider.py b/tests/unit/test_contrib_claude_provider.py index cb806e8..1f6bdbc 100644 --- a/tests/unit/test_contrib_claude_provider.py +++ b/tests/unit/test_contrib_claude_provider.py @@ -55,7 +55,7 @@ def _plan( bottle_json: dict = {"agent_provider": {"template": "claude"}} # type: ignore if supervise: bottle_json["supervise"] = True - manifest = ManifestIndex.from_json_obj({ + index = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": { "demo": { @@ -64,9 +64,10 @@ def _plan( "bottle": "dev", }, }, - }).load_for_agent("demo") + }) + manifest = index.load_for_agent("demo") spec = BottleSpec( - manifest=manifest, agent_name="demo", + manifest=index, agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", ) supervise_plan = None @@ -78,6 +79,7 @@ def _plan( ) return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=Path("/tmp/stage"), slug="demo-abc12", forwarded_env={}, diff --git a/tests/unit/test_contrib_codex_provider.py b/tests/unit/test_contrib_codex_provider.py index 8ba5b97..0d678c2 100644 --- a/tests/unit/test_contrib_codex_provider.py +++ b/tests/unit/test_contrib_codex_provider.py @@ -55,7 +55,7 @@ def _plan( bottle_json: dict = {"agent_provider": {"template": "codex"}} # type: ignore if supervise: bottle_json["supervise"] = True - manifest = ManifestIndex.from_json_obj({ + index = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": { "demo": { @@ -64,9 +64,10 @@ def _plan( "bottle": "dev", }, }, - }).load_for_agent("demo") + }) + manifest = index.load_for_agent("demo") spec = BottleSpec( - manifest=manifest, agent_name="demo", + manifest=index, agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", ) supervise_plan = None @@ -78,6 +79,7 @@ def _plan( ) return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=Path("/tmp/stage"), slug="demo-abc12", forwarded_env={}, diff --git a/tests/unit/test_contrib_pi_provider.py b/tests/unit/test_contrib_pi_provider.py index f9d83c2..0abe959 100644 --- a/tests/unit/test_contrib_pi_provider.py +++ b/tests/unit/test_contrib_pi_provider.py @@ -43,7 +43,7 @@ def _plan( skills: list[str] | None = None, agent_provision: AgentProvisionPlan | None = None, ) -> DockerBottlePlan: - manifest = ManifestIndex.from_json_obj({ + index = ManifestIndex.from_json_obj({ "bottles": {"dev": {"agent_provider": {"template": "pi"}}}, "agents": { "demo": { @@ -52,13 +52,15 @@ def _plan( "bottle": "dev", }, }, - }).load_for_agent("demo") + }) + manifest = index.load_for_agent("demo") spec = BottleSpec( - manifest=manifest, agent_name="demo", + manifest=index, agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", ) return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=Path("/tmp/stage"), slug="demo-abc12", forwarded_env={}, diff --git a/tests/unit/test_docker_launch_teardown.py b/tests/unit/test_docker_launch_teardown.py index 2a765c9..983bbc6 100644 --- a/tests/unit/test_docker_launch_teardown.py +++ b/tests/unit/test_docker_launch_teardown.py @@ -23,22 +23,17 @@ from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan from bot_bottle.manifest import ManifestIndex - -from bot_bottle.manifest import Manifest, ManifestIndex - - -def _manifest() -> Manifest: - return ManifestIndex.from_json_obj({ - "bottles": {"dev": {}}, - "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }).load_for_agent("demo") +_INDEX = ManifestIndex.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, +}) def _plan(tmp: str) -> DockerBottlePlan: stage = Path(tmp) - manifest = _manifest() + manifest = _INDEX.load_for_agent("demo") spec = BottleSpec( - manifest=manifest, + manifest=_INDEX, agent_name="demo", copy_cwd=False, user_cwd=tmp, @@ -46,6 +41,7 @@ def _plan(tmp: str) -> DockerBottlePlan: ) return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage, git_gate_plan=GitGatePlan( slug="test-teardown-00001", diff --git a/tests/unit/test_docker_provision_git_user.py b/tests/unit/test_docker_provision_git_user.py index 4c572df..2fb7466 100644 --- a/tests/unit/test_docker_provision_git_user.py +++ b/tests/unit/test_docker_provision_git_user.py @@ -51,16 +51,18 @@ def _plan(*, git_user: dict | None = None, # type: ignore bottle_json: dict = {} # type: ignore if git_user is not None: bottle_json["git-gate"] = {"user": git_user} - manifest = ManifestIndex.from_json_obj({ + index = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }).load_for_agent("demo") + }) + manifest = index.load_for_agent("demo") spec = BottleSpec( - manifest=manifest, agent_name="demo", + manifest=index, agent_name="demo", copy_cwd=copy_cwd, user_cwd=user_cwd, ) return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage_dir or Path("/tmp/stage"), slug="demo-abc12", forwarded_env={}, diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index 948eadc..2fbb373 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -11,6 +11,12 @@ from unittest.mock import patch from bot_bottle.backend.macos_container import launch from bot_bottle.backend.macos_container.bottle_plan import MacosContainerBottlePlan +from bot_bottle.manifest import ManifestIndex + +_MANIFEST = ManifestIndex.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, +}).load_for_agent("demo") def _plan( @@ -67,6 +73,7 @@ def _plan( ) return cast(MacosContainerBottlePlan, SimpleNamespace( spec=SimpleNamespace(), + manifest=_MANIFEST, stage_dir=stage_dir, slug="dev-abc", container_name="bot-bottle-dev-abc", @@ -193,6 +200,7 @@ class TestMacosContainerLaunchArgv(unittest.TestCase): ) plan = MacosContainerBottlePlan( spec=base.spec, + manifest=base.manifest, stage_dir=base.stage_dir, git_gate_plan=base.git_gate_plan, egress_plan=base.egress_plan, diff --git a/tests/unit/test_plan_print_parity.py b/tests/unit/test_plan_print_parity.py index f80b079..f8b5648 100644 --- a/tests/unit/test_plan_print_parity.py +++ b/tests/unit/test_plan_print_parity.py @@ -22,16 +22,15 @@ from bot_bottle.git_gate import GitGatePlan, GitGateUpstream from bot_bottle.manifest import Manifest, ManifestIndex -def _manifest() -> Manifest: - return ManifestIndex.from_json_obj({ - "bottles": {"dev": {}}, - "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }).load_for_agent("demo") +_INDEX = ManifestIndex.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, +}) -def _spec(manifest: Manifest, tmp: str) -> BottleSpec: +def _spec(index: ManifestIndex, tmp: str) -> BottleSpec: return BottleSpec( - manifest=manifest, + manifest=index, agent_name="demo", copy_cwd=False, user_cwd=tmp, @@ -92,10 +91,11 @@ def _agent_provision(tmp: str) -> AgentProvisionPlan: ) -def _docker_plan(spec: BottleSpec, tmp: str) -> DockerBottlePlan: +def _docker_plan(spec: BottleSpec, manifest: Manifest, tmp: str) -> DockerBottlePlan: stage = Path(tmp) return DockerBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage, git_gate_plan=_git_gate_plan(tmp), egress_plan=_egress_plan(tmp), @@ -107,10 +107,11 @@ def _docker_plan(spec: BottleSpec, tmp: str) -> DockerBottlePlan: ) -def _smolmachines_plan(spec: BottleSpec, tmp: str) -> SmolmachinesBottlePlan: +def _smolmachines_plan(spec: BottleSpec, manifest: Manifest, tmp: str) -> SmolmachinesBottlePlan: stage = Path(tmp) return SmolmachinesBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage, git_gate_plan=_git_gate_plan(tmp), egress_plan=_egress_plan(tmp), @@ -140,10 +141,10 @@ class TestGitGatePrintParity(unittest.TestCase): def setUp(self) -> None: self._tmp = tempfile.mkdtemp(prefix="plan-print-parity-") - manifest = _manifest() - spec = _spec(manifest, self._tmp) - self._docker_lines = _capture_print(_docker_plan(spec, self._tmp)) - self._smol_lines = _capture_print(_smolmachines_plan(spec, self._tmp)) + manifest = _INDEX.load_for_agent("demo") + spec = _spec(_INDEX, self._tmp) + self._docker_lines = _capture_print(_docker_plan(spec, manifest, self._tmp)) + self._smol_lines = _capture_print(_smolmachines_plan(spec, manifest, self._tmp)) def _git_gate_lines(self, lines: list[str]) -> list[str]: return [ln for ln in lines if "git gate" in ln] @@ -170,10 +171,10 @@ class TestEgressPrintParity(unittest.TestCase): def setUp(self) -> None: self._tmp = tempfile.mkdtemp(prefix="plan-print-parity-") - manifest = _manifest() - spec = _spec(manifest, self._tmp) - self._docker_lines = _capture_print(_docker_plan(spec, self._tmp)) - self._smol_lines = _capture_print(_smolmachines_plan(spec, self._tmp)) + manifest = _INDEX.load_for_agent("demo") + spec = _spec(_INDEX, self._tmp) + self._docker_lines = _capture_print(_docker_plan(spec, manifest, self._tmp)) + self._smol_lines = _capture_print(_smolmachines_plan(spec, manifest, self._tmp)) def _egress_section(self, lines: list[str]) -> list[str]: """Return lines from the egress label through the last route entry. diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index 5864916..e3fbbfc 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -110,7 +110,7 @@ def _plan( bottle_json["git-gate"] = git_gate_json if supervise: bottle_json["supervise"] = True - manifest = ManifestIndex.from_json_obj({ + index = ManifestIndex.from_json_obj({ "bottles": {"dev": bottle_json}, "agents": { "demo": { @@ -119,9 +119,10 @@ def _plan( "bottle": "dev", }, }, - }).load_for_agent("demo") + }) + manifest = index.load_for_agent("demo") spec = BottleSpec( - manifest=manifest, + manifest=index, agent_name="demo", copy_cwd=copy_cwd, user_cwd=user_cwd, @@ -135,6 +136,7 @@ def _plan( ) return SmolmachinesBottlePlan( spec=spec, + manifest=manifest, stage_dir=stage_dir or Path("/tmp/stage"), slug="demo-abc12", bundle_subnet="192.168.50.0/24", -- 2.52.0