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 e82dbaba09
commit 1ba185d1e0
14 changed files with 791 additions and 80 deletions
+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()