feat(#269): separate agent and bottle selection at launch time

- `bottle:` in agent frontmatter is now optional; agents without it
  are portable and require bottles to be selected at launch.
- Adds `filter_multiselect` to `tui.py`: multi-select picker with
  ordered selection list, Space/Enter to toggle, Ctrl-D to confirm.
- `ManifestIndex` gains `all_bottle_names` and `load_for_agent` accepts
  `bottle_names: tuple[str, ...]` to merge bottles in order at runtime.
- `merge_bottles_runtime` in `manifest_extends.py` applies the same
  field-merge rules as `extends:` to pre-resolved bottle objects.
- `BottleSpec` gains `bottle_names`; `_validate` and `write_launch_metadata`
  thread it through so `resume` replays the same bottle configuration.
- `cmd_start` shows the bottle multiselect after agent selection,
  pre-populated from the agent's `bottle:` field when present.
- Existing agents with `bottle:` declared continue to work unchanged.
This commit is contained in:
2026-06-25 06:55:06 +00:00
committed by didericis
parent 637ab4e89a
commit c28f3609fc
14 changed files with 791 additions and 80 deletions
+13 -3
View File
@@ -72,6 +72,9 @@ class BottleSpec:
identity: str = ""
label: str = ""
color: str = ""
# Ordered bottle names selected at launch (issue #269). When non-empty
# they are merged in order and replace the agent's `bottle:` field.
bottle_names: tuple[str, ...] = ()
@dataclass(frozen=True)
@@ -129,7 +132,11 @@ class BottlePlan(ABC):
info(f"provider : {self.agent_provision.template}")
print_multi("env ", env_names)
print_multi("skills ", list(agent.skills))
info(f"bottle : {agent.bottle}")
effective_bottles = (
list(spec.bottle_names) if spec.bottle_names
else ([agent.bottle] if agent.bottle else [])
)
print_multi("bottle ", effective_bottles)
identity = manifest.git_identity_summary()
if identity:
@@ -363,7 +370,7 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
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)
manifest = spec.manifest.load_for_agent(spec.agent_name, spec.bottle_names)
self._validate_skills(manifest.agent.skills)
self._validate_agent_provider_dockerfile(spec, manifest)
return manifest
@@ -389,9 +396,12 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
if not path.is_absolute():
path = Path(spec.user_cwd) / path
if not path.is_file():
effective = (
", ".join(spec.bottle_names) if spec.bottle_names else manifest.agent.bottle
)
die(
f"agent_provider.dockerfile for bottle "
f"'{manifest.agent.bottle}' not found: {path}"
f"'{effective}' not found: {path}"
)
@abstractmethod
+1
View File
@@ -63,6 +63,7 @@ def write_launch_metadata(
backend=backend,
label=spec.label,
color=spec.color,
bottle_names=spec.bottle_names,
))
+9
View File
@@ -112,6 +112,10 @@ class BottleMetadata:
backend: str = ""
label: str = ""
color: str = ""
# Ordered bottle names selected at launch (issue #269). Empty tuple
# for state dirs written before this change; resume falls back to
# the agent's `bottle:` field in that case.
bottle_names: tuple[str, ...] = ()
def metadata_path(identity: str) -> Path:
@@ -139,6 +143,10 @@ def read_metadata(identity: str) -> BottleMetadata | None:
if not isinstance(raw, dict):
return None
raw_typed = cast(dict[str, object], raw)
raw_bottle_names = raw_typed.get("bottle_names", [])
bottle_names: tuple[str, ...] = ()
if isinstance(raw_bottle_names, list):
bottle_names = tuple(str(n) for n in raw_bottle_names if isinstance(n, str))
return BottleMetadata(
identity=str(raw_typed.get("identity", identity)),
agent_name=str(raw_typed.get("agent_name", "")),
@@ -149,6 +157,7 @@ def read_metadata(identity: str) -> BottleMetadata | None:
backend=str(raw_typed.get("backend", "")),
label=str(raw_typed.get("label", "")),
color=str(raw_typed.get("color", "")),
bottle_names=bottle_names,
)
+1
View File
@@ -50,6 +50,7 @@ def cmd_resume(argv: list[str]) -> int:
copy_cwd=metadata.copy_cwd,
user_cwd=metadata.cwd or USER_CWD,
identity=metadata.identity,
bottle_names=tuple(metadata.bottle_names),
)
backend_name = metadata.backend or None
return _launch_bottle(
+47
View File
@@ -74,6 +74,20 @@ def cmd_start(argv: list[str]) -> int:
backend_name: str | None = args.backend
# Bottle multiselect: always show after agent selection so operators
# can compose bottles at launch time without editing agent manifests.
available_bottles = manifest.all_bottle_names
initial_bottle = _peek_agent_bottle(manifest, agent_name)
initial_bottles = [initial_bottle] if initial_bottle else []
selected_bottles = tui.filter_multiselect(
available_bottles,
title="Select bottles",
initial=initial_bottles,
)
if selected_bottles is None:
return 0
bottle_names = tuple(selected_bottles)
label, color = tui.name_color_modal(default_label=agent_name)
label, color = _resolve_unique_label(label, color)
@@ -84,6 +98,7 @@ def cmd_start(argv: list[str]) -> int:
user_cwd=USER_CWD,
label=label,
color=color,
bottle_names=bottle_names,
)
return _launch_bottle(
spec,
@@ -190,6 +205,38 @@ def _identity_from_plan(plan: object) -> str:
return getattr(plan, "slug", "")
def _peek_agent_bottle(manifest: ManifestIndex, agent_name: str) -> str:
"""Return the `bottle:` value from the named agent's frontmatter without
fully parsing the agent file, or "" when absent or unreadable.
Used to pre-populate the bottle multiselect with the agent's default
bottle so operators who haven't removed `bottle:` from their manifests
don't need to re-select it every time."""
if manifest.home_md is None:
# Eager mode (from_json_obj): agent is pre-parsed.
if agent_name in manifest.agents:
return manifest.agents[agent_name].bottle
return ""
from ..manifest_loader import scan_agent_names
from ..yaml_subset import YamlSubsetError, parse_frontmatter
home_agents = scan_agent_names(manifest.home_md / "agents")
cwd_agents: dict[str, Path] = {}
if manifest.cwd_md is not None:
cwd_agents = scan_agent_names(manifest.cwd_md / "agents")
merged = {**home_agents, **cwd_agents}
path = merged.get(agent_name)
if path is None:
return ""
try:
fm, _ = parse_frontmatter(path.read_text())
bottle = fm.get("bottle", "")
return str(bottle) if isinstance(bottle, str) else ""
except (OSError, YamlSubsetError):
return ""
def _resolve_unique_label(label: str, color: str) -> tuple[str, str]:
"""Re-prompt with a disclaimer until the label's slug is not already
in use among running bottles. Passes through unchanged when no
+209
View File
@@ -17,6 +17,42 @@ import sys
from typing import Any, Optional
def filter_multiselect(
items: list[str],
*,
title: str = "",
initial: Optional[list[str]] = None,
tty_path: str = "/dev/tty",
) -> Optional[list[str]]:
"""Render a multi-select picker over *items*.
Returns the ordered list of selected items, or ``None`` if the user
cancelled (Esc / ``q`` / Ctrl-C / Ctrl-D with no items).
Press Space or Enter to toggle the item under the cursor.
Press Ctrl-D to confirm the current selection (returns even if empty).
Press Esc/q to cancel (returns None).
*initial* pre-populates the selection in insertion order. Items
added are appended; removed items leave the remaining order unchanged.
"""
if not items:
return []
try:
tty_fd = open(tty_path, "r+b", buffering=0)
except OSError:
return None
try:
fd_dup = os.dup(tty_fd.fileno())
return _run_multiselect(
items, title=title, initial=list(initial or []), tty_fd=fd_dup
)
finally:
tty_fd.close()
def filter_select(
items: list[str],
*,
@@ -221,6 +257,179 @@ def _addstr_safe(screen: Any, row: int, col: int, text: str, attr: int = curses.
pass
# ---------------------------------------------------------------------------
# filter_multiselect internals
# ---------------------------------------------------------------------------
_KEY_SPACE = 32
def _run_multiselect(
items: list[str], *, title: str, initial: list[str], tty_fd: int
) -> Optional[list[str]]:
"""Drive a curses multi-select session on *tty_fd*."""
os.environ.setdefault("TERM", "xterm-256color")
orig_stdin = sys.__stdin__
orig_stdout = sys.__stdout__
try:
import io
tty_text = io.TextIOWrapper(io.FileIO(tty_fd, mode='r+'), write_through=True)
sys.__stdin__ = tty_text # type: ignore[assignment]
sys.__stdout__ = tty_text # type: ignore[assignment]
screen = curses.initscr()
curses.noecho()
curses.cbreak()
screen.keypad(True)
try:
result = _multiselect_loop(screen, items, title=title, initial=initial)
finally:
screen.keypad(False)
curses.nocbreak()
curses.echo()
curses.endwin()
except Exception: # noqa: W0718
return None
finally:
sys.__stdin__ = orig_stdin # type: ignore[assignment]
sys.__stdout__ = orig_stdout # type: ignore[assignment]
return result
def _multiselect_loop(
screen: Any, items: list[str], *, title: str, initial: list[str]
) -> Optional[list[str]]:
query = ""
cursor = 0
selected: list[str] = [s for s in initial if s in items]
while True:
filtered = _filter_items(items, query)
if not filtered:
cursor = 0
elif cursor >= len(filtered):
cursor = len(filtered) - 1
try:
_render_multiselect(screen, filtered, cursor, query=query, title=title, selected=selected)
except curses.error:
return None
try:
key = screen.getch()
except KeyboardInterrupt:
return None
if key in (_KEY_ESC, _KEY_CTRL_C, ord("q")):
return None
if key == _KEY_CTRL_D:
return list(selected)
if key in (curses.KEY_ENTER, _KEY_ENTER_ALT, ord("\r"), _KEY_SPACE):
if filtered:
item = filtered[cursor]
if item in selected:
selected.remove(item)
else:
selected.append(item)
elif key in (curses.KEY_UP, ord("k")):
if cursor > 0:
cursor -= 1
elif key in (curses.KEY_DOWN, ord("j")):
if cursor < len(filtered) - 1:
cursor += 1
elif key in (curses.KEY_BACKSPACE, _KEY_BACKSPACE_WIN, 127):
query = query[:-1]
new_filtered = _filter_items(items, query)
if cursor >= len(new_filtered):
cursor = max(0, len(new_filtered) - 1)
elif 32 <= key <= 126 and key != _KEY_SPACE:
query += chr(key)
cursor = 0
def _render_multiselect(
screen: Any,
filtered: list[str],
cursor: int,
*,
query: str,
title: str,
selected: list[str],
) -> None:
screen.erase()
rows, cols = screen.getmaxyx()
min_rows = 7
if rows < min_rows:
raise curses.error("terminal too small")
row = 0
if title and row < rows - 1:
_addstr_safe(screen, row, 0, title[:cols - 1], curses.A_BOLD)
row += 1
filter_label = f"Filter: {query}"
if row < rows - 1:
_addstr_safe(screen, row, 0, filter_label[:cols - 1])
row += 1
sep = "" * min(cols - 1, 40)
if row < rows - 1:
_addstr_safe(screen, row, 0, sep)
row += 1
# Reserve rows for: sep + selected-line + sep + help-line = 4
list_start = row
list_rows = rows - list_start - 4
if list_rows < 1:
return
selected_set = set(selected)
scroll = max(0, cursor - list_rows + 1)
visible = filtered[scroll: scroll + list_rows]
for idx, item in enumerate(visible):
abs_idx = scroll + idx
mark = "[*]" if item in selected_set else "[ ]"
prefix = "> " if abs_idx == cursor else " "
line = (prefix + mark + " " + item)[:cols - 1]
attr = curses.A_REVERSE if abs_idx == cursor else curses.A_NORMAL
if row < rows - 4:
_addstr_safe(screen, row, 0, line, attr)
row += 1
if row < rows - 3:
_addstr_safe(screen, row, 0, sep)
row += 1
selected_summary = "Selected (in order): " + (", ".join(selected) if selected else "(none)")
if row < rows - 2:
_addstr_safe(screen, row, 0, selected_summary[:cols - 1])
row += 1
if row < rows - 1:
_addstr_safe(screen, row, 0, sep)
row += 1
help_line = "[↑↓/jk] move [Space/Enter] toggle [Ctrl-D] done [Esc/q] cancel"
if row < rows:
_addstr_safe(screen, min(rows - 1, row), 0, help_line[:cols - 1])
screen.refresh()
# ---------------------------------------------------------------------------
# name_color_modal — two-step label + color picker
# ---------------------------------------------------------------------------
+102 -13
View File
@@ -215,6 +215,65 @@ def _merge_git_user(
)
def _resolve_effective_bottle_eager(
agent_name: str,
agent: "ManifestAgent",
bottle_names: "tuple[str, ...]",
bottles: "Mapping[str, ManifestBottle]",
) -> "ManifestBottle":
"""Return the effective ManifestBottle for the eager (from_json_obj) path.
When bottle_names is non-empty they are merged in order. When empty, falls
back to agent.bottle. Raises ManifestError when neither is set."""
from .manifest_extends import merge_bottles_runtime
if bottle_names:
resolved: list[ManifestBottle] = []
for bn in bottle_names:
if bn not in bottles:
available = ", ".join(sorted(bottles.keys())) or "(none)"
raise ManifestError(
f"bottle '{bn}' not defined. Available: {available}"
)
resolved.append(bottles[bn])
return merge_bottles_runtime(resolved)
if not agent.bottle:
raise ManifestError(
f"agent '{agent_name}' has no 'bottle' field and no bottles were "
f"selected at launch. Select at least one bottle or add "
f"'bottle: <name>' to the agent manifest."
)
return bottles[agent.bottle]
def _resolve_effective_bottle_lazy(
agent_name: str,
agent_bottle: str,
bottle_names: "tuple[str, ...]",
bottles_dir: "Path",
) -> "ManifestBottle":
"""Return the effective ManifestBottle for the lazy (from_md_dirs) path.
When bottle_names is non-empty they are resolved from disk and merged in
order. When empty, falls back to agent_bottle. Raises ManifestError when
neither is set."""
from .manifest_extends import merge_bottles_runtime
from .manifest_loader import load_bottle_chain_from_dir
if bottle_names:
resolved = [load_bottle_chain_from_dir(bn, bottles_dir) for bn in bottle_names]
return merge_bottles_runtime(resolved)
if not agent_bottle:
raise ManifestError(
f"agent '{agent_name}' has no 'bottle' field and no bottles were "
f"selected at launch. Select at least one bottle or add "
f"'bottle: <name>' to the agent manifest."
)
return load_bottle_chain_from_dir(agent_bottle, bottles_dir)
@dataclass(frozen=True)
class Manifest:
"""Single-agent/bottle value type. Returned by ManifestIndex.load_for_agent().
@@ -360,6 +419,18 @@ class ManifestIndex:
}
return cls(bottles=bottles, agents=agents)
@property
def all_bottle_names(self) -> list[str]:
"""Sorted list of all discoverable bottle names.
In names-only mode (from resolve/from_md_dirs) this scans bottle
filenames without reading their content. In eager mode (from
from_json_obj) it returns the pre-parsed bottles' names."""
if self.home_md is not None:
from .manifest_loader import scan_bottle_names
return scan_bottle_names(self.home_md / "bottles")
return sorted(self.bottles.keys())
@property
def all_agent_names(self) -> list[str]:
"""Sorted list of all discoverable agent names.
@@ -376,9 +447,18 @@ class ManifestIndex:
return sorted(home_names | cwd_names)
return sorted(self.agents.keys())
def load_for_agent(self, agent_name: str) -> "Manifest":
def load_for_agent(
self,
agent_name: str,
bottle_names: "tuple[str, ...] | None" = None,
) -> "Manifest":
"""Parse the named agent and its bottle; return a single-value Manifest.
`bottle_names` is an ordered list of bottles selected at launch time.
When non-empty they are resolved and merged in order (index 0 = base;
later entries override). When empty or None, falls back to the agent's
own `bottle:` field. Raises ManifestError when neither is set.
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
@@ -389,6 +469,8 @@ class ManifestIndex:
Always raises ManifestError if the agent is unknown or invalid.
Backends call this at preflight inside _validate."""
effective_bottle_names: tuple[str, ...] = bottle_names or ()
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
@@ -399,7 +481,9 @@ class ManifestIndex:
f"agent '{agent_name}' not defined. Available: {available}"
)
agent = self.agents[agent_name]
raw_bottle = self.bottles[agent.bottle]
raw_bottle = _resolve_effective_bottle_eager(
agent_name, agent, effective_bottle_names, self.bottles
)
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)
@@ -431,26 +515,31 @@ class ManifestIndex:
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).
# Determine the effective bottle name(s).
agent_bottle = fm.get("bottle") or ""
bottles_dir = self.home_md / "bottles"
raw_bottle = load_bottle_chain_from_dir(bottle_name, bottles_dir)
raw_bottle = _resolve_effective_bottle_lazy(
agent_name, str(agent_bottle), effective_bottle_names, bottles_dir
)
effective_bottle_name = (
effective_bottle_names[-1] if effective_bottle_names
else str(agent_bottle)
)
# Build and validate the full ManifestAgent.
agent_dict: dict[str, object] = {
"bottle": bottle_name,
"skills": fm.get("skills", []),
"prompt": body.strip(),
}
if agent_bottle:
agent_dict["bottle"] = agent_bottle
if "git-gate" in fm:
agent_dict["git-gate"] = fm["git-gate"]
agent = ManifestAgent.from_dict(agent_name, agent_dict, {bottle_name})
# Pass the effective bottle name as the known-bottles set so agents
# that have bottle: set are validated; agents without bottle: pass {}
# since bottle_names were already resolved above.
known = {effective_bottle_name} if effective_bottle_name else set()
agent = ManifestAgent.from_dict(agent_name, agent_dict, known)
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)
+16 -13
View File
@@ -109,7 +109,8 @@ class ManifestAgentProvider:
@dataclass(frozen=True)
class ManifestAgent:
bottle: str
# Optional: when empty the operator selects bottles at launch time.
bottle: str = ""
skills: tuple[str, ...] = ()
prompt: str = ""
# Per-agent git identity (issue #94). Overlays the referenced
@@ -129,18 +130,20 @@ class ManifestAgent:
f"allowed keys are {allowed}."
)
bottle = d.get("bottle")
if not isinstance(bottle, str) or not bottle:
raise ManifestError(
f"agent '{name}' must declare a 'bottle' field naming a "
f"defined bottle"
)
if bottle not in bottle_names:
available = ", ".join(sorted(bottle_names)) or "(none defined)"
raise ManifestError(
f"agent '{name}' references bottle '{bottle}', which is not defined. "
f"Available: {available}"
)
bottle_raw = d.get("bottle")
bottle = ""
if bottle_raw is not None:
if not isinstance(bottle_raw, str) or not bottle_raw:
raise ManifestError(
f"agent '{name}' bottle must be a non-empty string when declared"
)
if bottle_raw not in bottle_names:
available = ", ".join(sorted(bottle_names)) or "(none defined)"
raise ManifestError(
f"agent '{name}' references bottle '{bottle_raw}', which is not defined. "
f"Available: {available}"
)
bottle = bottle_raw
skills: tuple[str, ...] = ()
skills_raw = d.get("skills")
+52
View File
@@ -9,6 +9,58 @@ if TYPE_CHECKING:
from .manifest_egress import ManifestEgressConfig
def merge_bottles_runtime(bottles: "list[ManifestBottle]") -> "ManifestBottle":
"""Merge an ordered list of pre-resolved ManifestBottle objects.
Index 0 is the base; each subsequent entry is applied on top using
the same field-merge rules as the file-based extends machinery:
env: dict merge, later wins; git_user: per-field overlay, later
wins on non-empty; git (repos): union by name, later wins; egress
routes: concatenate; agent_provider, supervise: later replaces.
"""
if not bottles:
raise ValueError("merge_bottles_runtime requires at least one bottle")
result = bottles[0]
for override in bottles[1:]:
result = _merge_two_bottles_runtime(result, override)
return result
def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle") -> "ManifestBottle":
from .manifest import ManifestBottle, ManifestGitUser
from .manifest_egress import ManifestEgressConfig
merged_env = {**base.env, **override.env}
merged_git_user = ManifestGitUser(
name=override.git_user.name or base.git_user.name,
email=override.git_user.email or base.git_user.email,
)
# git repos: union keyed by Name, override wins per-name.
base_repos_by_name = {entry.Name: entry for entry in base.git}
override_repos_by_name = {entry.Name: entry for entry in override.git}
merged_repos_names = list(base_repos_by_name) + [
n for n in override_repos_by_name if n not in base_repos_by_name
]
merged_git = tuple(
override_repos_by_name.get(n, base_repos_by_name[n])
for n in merged_repos_names
)
merged_routes: tuple[ManifestEgressRoute, ...] = base.egress.routes + override.egress.routes
merged_egress = ManifestEgressConfig(routes=merged_routes, Log=override.egress.Log)
return ManifestBottle(
env=merged_env,
agent_provider=override.agent_provider,
git=merged_git,
git_user=merged_git_user,
egress=merged_egress,
supervise=override.supervise,
)
def resolve_bottles(raws: dict[str, dict[str, object]]) -> dict[str, ManifestBottle]:
"""Apply `extends:` chains and return resolved ManifestBottle objects."""
cache: dict[str, ManifestBottle] = {}
+19
View File
@@ -32,6 +32,25 @@ def check_stale_json(dir_path: Path, md_dir: Path, label: str) -> None:
)
def scan_bottle_names(bottles_dir: Path) -> list[str]:
"""Scan `<bottles_dir>/*.md` for valid filenames and return sorted bottle names.
No file content is read. Invalid filenames are skipped with a warning."""
result: list[str] = []
if not bottles_dir.is_dir():
return result
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
result.append(name)
return result
def scan_agent_names(agents_dir: Path) -> dict[str, Path]:
"""Scan `<agents_dir>/*.md` for valid filenames and return `{name: path}`.
+2 -2
View File
@@ -18,8 +18,8 @@ _FILENAME_RX = re.compile(r"^[a-z][a-z0-9-]*$")
BOTTLE_KEYS = frozenset(
{"env", "extends", "agent_provider", "git-gate", "egress", "supervise"}
)
AGENT_KEYS_REQUIRED = frozenset({"bottle"})
AGENT_KEYS_OPTIONAL = frozenset({"skills", "git-gate"})
AGENT_KEYS_REQUIRED: frozenset[str] = frozenset()
AGENT_KEYS_OPTIONAL = frozenset({"bottle", "skills", "git-gate"})
# Claude Code subagent fields bot-bottle ignores at launch but does
# not reject. This lets the same file double as
+107 -47
View File
@@ -1,7 +1,8 @@
"""Unit: cmd_start selector dispatch (PRD 0051).
"""Unit: cmd_start selector dispatch (PRD 0051, issue #269).
Tests that cmd_start calls filter_select only when the agent name is
absent, skips it when the agent is explicit, and returns 0 on cancel.
absent, shows the bottle multiselect after agent selection, and skips
pickers when both are explicitly set.
All actual launch work is stubbed so no container is created.
"""
@@ -10,17 +11,23 @@ from __future__ import annotations
import os
import unittest
from unittest.mock import MagicMock, patch
from unittest.mock import MagicMock, call, patch
import bot_bottle.cli.start as start_mod
import bot_bottle.cli.tui as tui_mod
from bot_bottle.backend import ActiveAgent
def _make_manifest(agent_names: list[str]):
def _make_manifest(
agent_names: list[str],
bottle_names: list[str] | None = None,
agent_bottle: str = "",
):
manifest = MagicMock()
manifest.agents = {name: MagicMock() for name in agent_names}
manifest.agents = {name: MagicMock(bottle=agent_bottle) for name in agent_names}
manifest.all_agent_names = sorted(agent_names)
manifest.all_bottle_names = sorted(bottle_names or [])
manifest.home_md = None # eager mode so _peek_agent_bottle uses agents dict
return manifest
@@ -28,27 +35,27 @@ class TestCmdStartSelector(unittest.TestCase):
"""Drive cmd_start with a minimal set of stubs."""
def setUp(self):
# Stub Manifest.resolve so no on-disk manifest is needed.
self._manifest = _make_manifest(["researcher", "implementer"])
self._manifest = _make_manifest(["researcher", "implementer"], ["claude", "dev"])
self._resolve_patch = patch(
"bot_bottle.cli.start.ManifestIndex.resolve",
return_value=self._manifest,
)
self._resolve_patch.start()
# Stub _launch_bottle so no real container work happens.
self._launch_patch = patch(
"bot_bottle.cli.start._launch_bottle",
return_value=0,
)
self._launch_mock = self._launch_patch.start()
# Stub filter_select to avoid opening /dev/tty.
self._tui_patch = patch.object(tui_mod, "filter_select")
self._tui_mock = self._tui_patch.start()
# Stub filter_select (agent picker) and filter_multiselect (bottle picker).
self._agent_picker_patch = patch.object(tui_mod, "filter_select")
self._agent_picker_mock = self._agent_picker_patch.start()
self._bottle_picker_patch = patch.object(tui_mod, "filter_multiselect")
self._bottle_picker_mock = self._bottle_picker_patch.start()
self._bottle_picker_mock.return_value = ["claude"] # default: one bottle selected
# Ensure BOT_BOTTLE_BACKEND is absent so omitted --backend
# flows through to the resolver default.
self._env_patch = patch.dict(os.environ, {}, clear=False)
self._env_patch.start()
os.environ.pop("BOT_BOTTLE_BACKEND", None)
@@ -56,50 +63,108 @@ class TestCmdStartSelector(unittest.TestCase):
def tearDown(self):
self._resolve_patch.stop()
self._launch_patch.stop()
self._tui_patch.stop()
self._agent_picker_patch.stop()
self._bottle_picker_patch.stop()
self._env_patch.stop()
# ------------------------------------------------------------------
# Both explicit — no picker shown
# Agent explicit — agent picker skipped; bottle picker always shown
# ------------------------------------------------------------------
def test_both_explicit_skips_picker(self):
self._tui_mock.return_value = "researcher"
def test_explicit_agent_skips_agent_picker(self):
rc = start_mod.cmd_start(["--backend=docker", "researcher"])
self.assertEqual(0, rc)
self._tui_mock.assert_not_called()
self._agent_picker_mock.assert_not_called()
self._bottle_picker_mock.assert_called_once()
self._launch_mock.assert_called_once()
_, kwargs = self._launch_mock.call_args
self.assertEqual("docker", kwargs["backend_name"])
def test_explicit_agent_bottle_picker_shows_available_bottles(self):
start_mod.cmd_start(["researcher"])
call_kwargs = self._bottle_picker_mock.call_args
self.assertEqual(["claude", "dev"], call_kwargs[0][0])
self.assertIn("bottle", call_kwargs[1]["title"].lower())
# ------------------------------------------------------------------
# Agent absent → agent picker fires; backend explicit
# Agent absent → agent picker fires; bottle picker always follows
# ------------------------------------------------------------------
def test_agent_absent_shows_agent_picker(self):
self._tui_mock.return_value = "researcher"
self._agent_picker_mock.return_value = "researcher"
rc = start_mod.cmd_start(["--backend=docker"])
self.assertEqual(0, rc)
self._tui_mock.assert_called_once()
call_kwargs = self._tui_mock.call_args
self._agent_picker_mock.assert_called_once()
call_kwargs = self._agent_picker_mock.call_args
self.assertEqual(["implementer", "researcher"], call_kwargs[0][0])
self.assertIn("agent", call_kwargs[1]["title"].lower())
# Bottle picker must also fire after agent selection.
self._bottle_picker_mock.assert_called_once()
def test_agent_picker_cancel_returns_0(self):
self._tui_mock.return_value = None
def test_agent_picker_cancel_skips_bottle_picker(self):
self._agent_picker_mock.return_value = None
rc = start_mod.cmd_start(["--backend=docker"])
self.assertEqual(0, rc)
self._bottle_picker_mock.assert_not_called()
self._launch_mock.assert_not_called()
def test_bottle_picker_cancel_returns_0(self):
self._bottle_picker_mock.return_value = None
rc = start_mod.cmd_start(["researcher"])
self.assertEqual(0, rc)
self._launch_mock.assert_not_called()
# ------------------------------------------------------------------
# Agent explicit, backend absent → no picker
# Bottle selection is forwarded to BottleSpec
# ------------------------------------------------------------------
def test_backend_absent_uses_default_without_picker(self):
rc = start_mod.cmd_start(["researcher"])
self.assertEqual(0, rc)
self._tui_mock.assert_not_called()
def test_selected_bottles_forwarded_to_spec(self):
self._bottle_picker_mock.return_value = ["claude", "dev"]
start_mod.cmd_start(["researcher"])
self._launch_mock.assert_called_once()
spec = self._launch_mock.call_args[0][0]
self.assertEqual(("claude", "dev"), spec.bottle_names)
def test_empty_bottle_selection_forwarded(self):
self._bottle_picker_mock.return_value = []
start_mod.cmd_start(["researcher"])
self._launch_mock.assert_called_once()
spec = self._launch_mock.call_args[0][0]
self.assertEqual((), spec.bottle_names)
# ------------------------------------------------------------------
# Agent default bottle pre-populates the picker
# ------------------------------------------------------------------
def test_agent_bottle_prepopulates_bottle_picker(self):
manifest = _make_manifest(
["implementer"], ["claude", "dev"], agent_bottle="claude"
)
with patch(
"bot_bottle.cli.start.ManifestIndex.resolve", return_value=manifest
):
start_mod.cmd_start(["implementer"])
call_kwargs = self._bottle_picker_mock.call_args
self.assertEqual(["claude"], call_kwargs[1]["initial"])
def test_no_agent_bottle_empty_initial(self):
manifest = _make_manifest(["researcher"], ["claude", "dev"], agent_bottle="")
with patch(
"bot_bottle.cli.start.ManifestIndex.resolve", return_value=manifest
):
start_mod.cmd_start(["researcher"])
call_kwargs = self._bottle_picker_mock.call_args
self.assertEqual([], call_kwargs[1]["initial"])
# ------------------------------------------------------------------
# Backend wiring
# ------------------------------------------------------------------
def test_explicit_backend_forwarded(self):
start_mod.cmd_start(["--backend=docker", "researcher"])
_, kwargs = self._launch_mock.call_args
self.assertEqual("docker", kwargs["backend_name"])
def test_absent_backend_uses_default(self):
start_mod.cmd_start(["researcher"])
_, kwargs = self._launch_mock.call_args
self.assertIsNone(kwargs["backend_name"])
@@ -110,28 +175,21 @@ class TestCmdStartSelector(unittest.TestCase):
finally:
os.environ.pop("BOT_BOTTLE_BACKEND", None)
self.assertEqual(0, rc)
self._tui_mock.assert_not_called()
# ------------------------------------------------------------------
# Both absent → only agent picker
# ------------------------------------------------------------------
def test_both_absent_shows_only_agent_picker(self):
self._tui_mock.return_value = "researcher"
def test_both_absent_shows_agent_picker_then_bottle_picker(self):
self._agent_picker_mock.return_value = "researcher"
rc = start_mod.cmd_start([])
self.assertEqual(0, rc)
self._tui_mock.assert_called_once()
title = self._tui_mock.call_args[1]["title"].lower()
self.assertIn("agent", title)
self._agent_picker_mock.assert_called_once()
self._bottle_picker_mock.assert_called_once()
self._launch_mock.assert_called_once()
_, kwargs = self._launch_mock.call_args
self.assertIsNone(kwargs["backend_name"])
def test_both_absent_agent_cancel_skips_backend_picker(self):
self._tui_mock.side_effect = [None]
def test_both_absent_agent_cancel_skips_bottle_and_launch(self):
self._agent_picker_mock.return_value = None
rc = start_mod.cmd_start([])
self.assertEqual(0, rc)
self.assertEqual(1, self._tui_mock.call_count)
self._agent_picker_mock.assert_called_once()
self._bottle_picker_mock.assert_not_called()
self._launch_mock.assert_not_called()
@@ -149,11 +207,13 @@ class TestCmdStartLabelCollision(unittest.TestCase):
"""cmd_start re-prompts when the label's slug is already running."""
def setUp(self):
self._manifest = _make_manifest(["researcher"])
self._manifest = _make_manifest(["researcher"], ["claude"])
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()
# Stub the bottle picker to always return a selection.
patch.object(tui_mod, "filter_multiselect", return_value=["claude"]).start()
self.addCleanup(patch.stopall)
def test_no_collision_proceeds_without_reprompt(self):
+13 -2
View File
@@ -1,4 +1,4 @@
"""Unit tests for bot_bottle.cli.tui — filter_select internals.
"""Unit tests for bot_bottle.cli.tui — filter_select and filter_multiselect.
We test the pure-Python logic (_filter_items, cursor movement, confirm,
cancel) by exercising the internal helpers directly, without spinning up
@@ -9,7 +9,7 @@ from __future__ import annotations
import unittest
from bot_bottle.cli.tui import _filter_items, filter_select
from bot_bottle.cli.tui import _filter_items, filter_multiselect, filter_select
class TestFilterItems(unittest.TestCase):
@@ -46,5 +46,16 @@ class TestFilterSelectEmptyItems(unittest.TestCase):
self.assertIsNone(result)
class TestFilterMultiselectEmptyItems(unittest.TestCase):
def test_returns_empty_list_for_empty_items(self):
# No TTY needed — short-circuits before opening tty.
result = filter_multiselect([], title="Select", tty_path="/dev/null")
self.assertEqual([], result)
def test_returns_none_when_tty_unavailable(self):
result = filter_multiselect(["a", "b"], tty_path="/nonexistent/tty")
self.assertIsNone(result)
if __name__ == "__main__":
unittest.main()
+200
View File
@@ -0,0 +1,200 @@
"""Unit: runtime bottle composition (issue #269).
Tests for merge_bottles_runtime and ManifestIndex.load_for_agent with
the new bottle_names parameter.
"""
from __future__ import annotations
import os
import shutil
import tempfile
import textwrap
import unittest
from pathlib import Path
from bot_bottle.manifest import ManifestBottle, ManifestError, ManifestIndex
from bot_bottle.manifest_extends import merge_bottles_runtime
def _index(bottles: dict, agents: dict) -> ManifestIndex:
return ManifestIndex.from_json_obj({"bottles": bottles, "agents": agents})
def _bottle(**kwargs) -> ManifestBottle:
return ManifestBottle.from_dict("test", kwargs)
class TestMergeBottlesRuntime(unittest.TestCase):
def test_single_bottle_returns_as_is(self):
b = _bottle(env={"FOO": "1"})
result = merge_bottles_runtime([b])
self.assertEqual({"FOO": "1"}, dict(result.env))
def test_env_later_wins(self):
base = _bottle(env={"FOO": "base", "ONLY_BASE": "x"})
override = _bottle(env={"FOO": "override", "ONLY_OVERRIDE": "y"})
result = merge_bottles_runtime([base, override])
self.assertEqual("override", result.env["FOO"])
self.assertEqual("x", result.env["ONLY_BASE"])
self.assertEqual("y", result.env["ONLY_OVERRIDE"])
def test_egress_routes_concatenated(self):
from bot_bottle.manifest_egress import ManifestEgressConfig, ManifestEgressRoute
r1 = ManifestEgressRoute(Host="api.a.com")
r2 = ManifestEgressRoute(Host="api.b.com")
base = ManifestBottle(egress=ManifestEgressConfig(routes=(r1,)))
override = ManifestBottle(egress=ManifestEgressConfig(routes=(r2,)))
result = merge_bottles_runtime([base, override])
hosts = [r.Host for r in result.egress.routes]
self.assertIn("api.a.com", hosts)
self.assertIn("api.b.com", hosts)
def test_supervise_later_wins(self):
base = _bottle(supervise=True)
override = _bottle(supervise=False)
result = merge_bottles_runtime([base, override])
self.assertFalse(result.supervise)
def test_three_bottles_merged_left_to_right(self):
b1 = _bottle(env={"A": "1", "B": "1", "C": "1"})
b2 = _bottle(env={"B": "2", "C": "2"})
b3 = _bottle(env={"C": "3"})
result = merge_bottles_runtime([b1, b2, b3])
self.assertEqual("1", result.env["A"])
self.assertEqual("2", result.env["B"])
self.assertEqual("3", result.env["C"])
def test_empty_list_raises(self):
with self.assertRaises(ValueError):
merge_bottles_runtime([])
class TestLoadForAgentWithBottleNames(unittest.TestCase):
def test_bottle_names_override_agent_bottle(self):
idx = _index(
bottles={
"base": {"env": {"X": "base"}},
"override": {"env": {"X": "override"}},
},
agents={"impl": {"bottle": "base", "skills": [], "prompt": ""}},
)
m = idx.load_for_agent("impl", ("override",))
self.assertEqual("override", m.bottle.env["X"])
def test_bottle_names_merged_in_order(self):
idx = _index(
bottles={
"a": {"env": {"X": "a", "A": "only-a"}},
"b": {"env": {"X": "b", "B": "only-b"}},
},
agents={"impl": {"bottle": "a", "skills": [], "prompt": ""}},
)
m = idx.load_for_agent("impl", ("a", "b"))
self.assertEqual("b", m.bottle.env["X"])
self.assertEqual("only-a", m.bottle.env["A"])
self.assertEqual("only-b", m.bottle.env["B"])
def test_empty_bottle_names_uses_agent_bottle(self):
idx = _index(
bottles={"base": {"env": {"X": "base"}}},
agents={"impl": {"bottle": "base", "skills": [], "prompt": ""}},
)
m = idx.load_for_agent("impl", ())
self.assertEqual("base", m.bottle.env["X"])
def test_no_bottle_and_no_bottle_names_raises(self):
idx = _index(
bottles={"base": {}},
agents={"impl": {"skills": [], "prompt": ""}},
)
with self.assertRaises(ManifestError) as ctx:
idx.load_for_agent("impl", ())
self.assertIn("no 'bottle' field", str(ctx.exception))
def test_unknown_bottle_name_raises(self):
idx = _index(
bottles={"base": {}},
agents={"impl": {"bottle": "base", "skills": [], "prompt": ""}},
)
with self.assertRaises(ManifestError) as ctx:
idx.load_for_agent("impl", ("nonexistent",))
self.assertIn("nonexistent", str(ctx.exception))
def test_agent_without_bottle_works_with_bottle_names(self):
idx = _index(
bottles={"base": {"env": {"X": "base"}}},
agents={"impl": {"skills": [], "prompt": ""}},
)
m = idx.load_for_agent("impl", ("base",))
self.assertEqual("base", m.bottle.env["X"])
class TestAllBottleNames(unittest.TestCase):
def test_eager_mode_returns_bottle_names(self):
idx = _index(
bottles={"alpha": {}, "beta": {}, "gamma": {}},
agents={"impl": {"bottle": "alpha", "skills": [], "prompt": ""}},
)
self.assertEqual(["alpha", "beta", "gamma"], idx.all_bottle_names)
def test_lazy_mode_scans_files(self):
home = Path(tempfile.mkdtemp(prefix="cb-home-"))
orig_home = os.environ.get("HOME")
os.environ["HOME"] = str(home)
try:
bottles_dir = home / ".bot-bottle" / "bottles"
agents_dir = home / ".bot-bottle" / "agents"
bottles_dir.mkdir(parents=True)
agents_dir.mkdir(parents=True)
(bottles_dir / "claude.md").write_text("---\n---\n")
(bottles_dir / "dev.md").write_text("---\n---\n")
(agents_dir / "impl.md").write_text("---\nbottle: claude\n---\n")
idx = ManifestIndex.resolve(str(home))
self.assertEqual(["claude", "dev"], idx.all_bottle_names)
finally:
if orig_home is None:
os.environ.pop("HOME", None)
else:
os.environ["HOME"] = orig_home
shutil.rmtree(home, ignore_errors=True)
class TestAgentOptionalBottleMd(unittest.TestCase):
"""Agent file without bottle: works when bottle_names are provided at launch."""
def setUp(self) -> None:
self.home = Path(tempfile.mkdtemp(prefix="cb-home-"))
self._orig_home = os.environ.get("HOME")
os.environ["HOME"] = str(self.home)
def tearDown(self) -> None:
if self._orig_home is None:
os.environ.pop("HOME", None)
else:
os.environ["HOME"] = self._orig_home
shutil.rmtree(self.home, ignore_errors=True)
def _write(self, rel: str, text: str) -> None:
p = self.home / ".bot-bottle" / rel
p.parent.mkdir(parents=True, exist_ok=True)
p.write_text(textwrap.dedent(text).lstrip("\n"))
def test_agent_without_bottle_resolves_with_bottle_names(self):
self._write("bottles/dev.md", "---\nenv:\n X: dev\n---\n")
self._write("agents/impl.md", "---\n---\nimpl agent.\n")
idx = ManifestIndex.resolve(str(self.home))
m = idx.load_for_agent("impl", ("dev",))
self.assertEqual("dev", m.bottle.env["X"])
def test_agent_without_bottle_fails_without_bottle_names(self):
self._write("bottles/dev.md", "---\n---\n")
self._write("agents/impl.md", "---\n---\nimpl agent.\n")
idx = ManifestIndex.resolve(str(self.home))
with self.assertRaises(ManifestError) as ctx:
idx.load_for_agent("impl", ())
self.assertIn("no 'bottle' field", str(ctx.exception))
if __name__ == "__main__":
unittest.main()