feat(tui,start): space/enter split, bottle lineage, YAML preflight
Three UX improvements requested in #270 review: - filter_multiselect: Space toggles selection, Enter confirms (was both) - bottle picker: bottles with extends chains show ancestry labels (e.g. 'claude-dev <- bot-bottle-dev <- dev') for at-a-glance lineage - preflight: replaces key-value summary with YAML of the resolved manifest
This commit is contained in:
+113
-8
@@ -32,7 +32,7 @@ from ..bottle_state import (
|
||||
mark_preserved,
|
||||
)
|
||||
from ..log import info
|
||||
from ..manifest import ManifestIndex
|
||||
from ..manifest import Manifest, ManifestIndex
|
||||
from ._common import PROG, USER_CWD, read_tty_line
|
||||
from . import tui
|
||||
|
||||
@@ -76,16 +76,19 @@ def cmd_start(argv: list[str]) -> int:
|
||||
# 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
|
||||
lineage_map = _bottle_lineage(manifest)
|
||||
display_labels = [lineage_map.get(n, n) for n in available_bottles]
|
||||
label_to_name = {lineage_map.get(n, n): n for n in available_bottles}
|
||||
initial_bottle = _peek_agent_bottle(manifest, agent_name)
|
||||
initial_bottles = [initial_bottle] if initial_bottle else []
|
||||
selected_bottles = tui.filter_multiselect(
|
||||
available_bottles,
|
||||
initial_labels = [lineage_map.get(initial_bottle, initial_bottle)] if initial_bottle else []
|
||||
selected_labels = tui.filter_multiselect(
|
||||
display_labels,
|
||||
title="Select bottles",
|
||||
initial=initial_bottles,
|
||||
initial=initial_labels,
|
||||
)
|
||||
if selected_bottles is None:
|
||||
if selected_labels is None:
|
||||
return 0
|
||||
bottle_names = tuple(selected_bottles)
|
||||
bottle_names = tuple(label_to_name.get(lbl, lbl) for lbl in selected_labels)
|
||||
|
||||
label, color = tui.name_color_modal(default_label=agent_name)
|
||||
label, color = _resolve_unique_label(label, color)
|
||||
@@ -262,10 +265,112 @@ def _text_prompt_yes() -> bool:
|
||||
|
||||
def _text_render_preflight():
|
||||
def _render(plan: DockerBottlePlan) -> None:
|
||||
plan.print()
|
||||
print(file=sys.stderr)
|
||||
print(_manifest_to_yaml(plan.manifest), file=sys.stderr)
|
||||
return _render
|
||||
|
||||
|
||||
def _bottle_lineage(manifest: ManifestIndex) -> dict[str, str]:
|
||||
"""Return {bottle_name: lineage_label} for bottles that have an extends chain.
|
||||
|
||||
Bottles without a parent are omitted (the caller falls back to the bare name).
|
||||
Labels show the chain root-first: e.g. 'claude-dev <- bot-bottle-dev <- dev'."""
|
||||
if manifest.home_md is None:
|
||||
return {}
|
||||
bottles_dir = manifest.home_md / "bottles"
|
||||
if not bottles_dir.is_dir():
|
||||
return {}
|
||||
|
||||
from ..yaml_subset import YamlSubsetError, parse_frontmatter
|
||||
|
||||
extends_of: dict[str, str] = {}
|
||||
for path in bottles_dir.glob("*.md"):
|
||||
try:
|
||||
fm, _ = parse_frontmatter(path.read_text())
|
||||
parent = fm.get("extends", "")
|
||||
if isinstance(parent, str) and parent:
|
||||
extends_of[path.stem] = parent
|
||||
except (OSError, YamlSubsetError):
|
||||
pass
|
||||
|
||||
labels: dict[str, str] = {}
|
||||
for name in extends_of:
|
||||
chain = [name]
|
||||
seen = {name}
|
||||
cur = name
|
||||
while cur in extends_of:
|
||||
par = extends_of[cur]
|
||||
if par in seen:
|
||||
break
|
||||
chain.append(par)
|
||||
seen.add(par)
|
||||
cur = par
|
||||
labels[name] = " <- ".join(reversed(chain))
|
||||
|
||||
return labels
|
||||
|
||||
|
||||
def _manifest_to_yaml(manifest: Manifest) -> str:
|
||||
"""Serialize the resolved Manifest to a YAML string for preflight display."""
|
||||
lines: list[str] = []
|
||||
|
||||
agent = manifest.agent
|
||||
lines.append("agent:")
|
||||
if agent.skills:
|
||||
lines.append(" skills:")
|
||||
for s in agent.skills:
|
||||
lines.append(f" - {s}")
|
||||
if not agent.git_user.is_empty():
|
||||
lines.append(" git-gate:")
|
||||
lines.append(" user:")
|
||||
if agent.git_user.name:
|
||||
lines.append(f" name: {agent.git_user.name}")
|
||||
if agent.git_user.email:
|
||||
lines.append(f" email: {agent.git_user.email}")
|
||||
|
||||
bottle = manifest.bottle
|
||||
lines.append("bottle:")
|
||||
|
||||
if bottle.agent_provider.template != "claude" or bottle.agent_provider.dockerfile:
|
||||
lines.append(" agent_provider:")
|
||||
lines.append(f" template: {bottle.agent_provider.template}")
|
||||
if bottle.agent_provider.dockerfile:
|
||||
lines.append(f" dockerfile: {bottle.agent_provider.dockerfile}")
|
||||
|
||||
if bottle.env:
|
||||
lines.append(" env:")
|
||||
for k, v in sorted(bottle.env.items()):
|
||||
lines.append(f" {k}: {v}")
|
||||
|
||||
has_git_gate = not bottle.git_user.is_empty() or bottle.git
|
||||
if has_git_gate:
|
||||
lines.append(" git-gate:")
|
||||
if not bottle.git_user.is_empty():
|
||||
lines.append(" user:")
|
||||
if bottle.git_user.name:
|
||||
lines.append(f" name: {bottle.git_user.name}")
|
||||
if bottle.git_user.email:
|
||||
lines.append(f" email: {bottle.git_user.email}")
|
||||
if bottle.git:
|
||||
lines.append(" repos:")
|
||||
for entry in bottle.git:
|
||||
lines.append(f" {entry.Name}:")
|
||||
lines.append(f" url: {entry.Upstream}")
|
||||
|
||||
if bottle.egress.routes:
|
||||
lines.append(" egress:")
|
||||
lines.append(" routes:")
|
||||
for r in bottle.egress.routes:
|
||||
lines.append(f" - host: {r.Host}")
|
||||
if r.AuthScheme:
|
||||
lines.append(f" auth:")
|
||||
lines.append(f" scheme: {r.AuthScheme}")
|
||||
|
||||
lines.append(f" supervise: {'true' if bottle.supervise else 'false'}")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _launch_bottle(
|
||||
spec: BottleSpec,
|
||||
*,
|
||||
|
||||
@@ -29,7 +29,8 @@ def filter_multiselect(
|
||||
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 Space to toggle the item under the cursor.
|
||||
Press Enter to confirm the current selection.
|
||||
Press Ctrl-D to confirm the current selection (returns even if empty).
|
||||
Press Esc/q to cancel (returns None).
|
||||
|
||||
@@ -356,7 +357,10 @@ def _multiselect_loop(
|
||||
continue
|
||||
|
||||
if focus == "filter":
|
||||
if key in (curses.KEY_ENTER, _KEY_ENTER_ALT, ord("\r"), _KEY_SPACE):
|
||||
if key in (curses.KEY_ENTER, _KEY_ENTER_ALT, ord("\r")):
|
||||
return list(selected)
|
||||
|
||||
elif key == _KEY_SPACE:
|
||||
if filtered:
|
||||
item = filtered[cursor]
|
||||
if item in selected:
|
||||
@@ -500,7 +504,7 @@ def _render_multiselect(
|
||||
row += 1
|
||||
|
||||
if focus == "filter":
|
||||
help_line = "[↑↓/jk] move [Space/Enter] toggle [Tab] reorder [Ctrl-D] done [Esc/q] cancel"
|
||||
help_line = "[↑↓/jk] move [Space] toggle [Enter] confirm [Tab] reorder [Esc/q] cancel"
|
||||
else:
|
||||
help_line = "[↑↓/jk] cursor [K/J] reorder [Space/Enter] remove [Tab] back [Ctrl-D] done"
|
||||
if row < rows:
|
||||
|
||||
@@ -253,5 +253,103 @@ class TestCmdStartLabelCollision(unittest.TestCase):
|
||||
self.assertIn("already in use", second_call_kwargs.get("disclaimer", ""))
|
||||
|
||||
|
||||
class TestBottleLineage(unittest.TestCase):
|
||||
"""Unit tests for _bottle_lineage."""
|
||||
|
||||
def test_returns_empty_in_eager_mode(self):
|
||||
manifest = _make_manifest(["agent"], ["base", "dev"])
|
||||
# home_md is None in eager mode → no file reads, returns {}
|
||||
result = start_mod._bottle_lineage(manifest)
|
||||
self.assertEqual({}, result)
|
||||
|
||||
def test_reads_extends_chain_from_files(self):
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
bottles_dir = Path(tmp) / "bottles"
|
||||
bottles_dir.mkdir()
|
||||
(bottles_dir / "base.md").write_text("---\n{}\n---\n")
|
||||
(bottles_dir / "mid.md").write_text("---\nextends: base\n---\n")
|
||||
(bottles_dir / "leaf.md").write_text("---\nextends: mid\n---\n")
|
||||
|
||||
manifest = MagicMock()
|
||||
manifest.home_md = Path(tmp)
|
||||
|
||||
result = start_mod._bottle_lineage(manifest)
|
||||
|
||||
self.assertNotIn("base", result) # no parent → not in map
|
||||
self.assertEqual("base <- mid", result["mid"])
|
||||
self.assertEqual("base <- mid <- leaf", result["leaf"])
|
||||
|
||||
def test_cycle_protection(self):
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
bottles_dir = Path(tmp) / "bottles"
|
||||
bottles_dir.mkdir()
|
||||
(bottles_dir / "a.md").write_text("---\nextends: b\n---\n")
|
||||
(bottles_dir / "b.md").write_text("---\nextends: a\n---\n")
|
||||
|
||||
manifest = MagicMock()
|
||||
manifest.home_md = Path(tmp)
|
||||
|
||||
result = start_mod._bottle_lineage(manifest)
|
||||
|
||||
# Cycle must not hang; each should get a two-element chain.
|
||||
for name in ("a", "b"):
|
||||
self.assertIn(name, result)
|
||||
self.assertIn("<-", result[name])
|
||||
|
||||
|
||||
class TestManifestToYaml(unittest.TestCase):
|
||||
"""Unit tests for _manifest_to_yaml."""
|
||||
|
||||
def _make_manifest_obj(self, *, skills=(), env=None, supervise=True,
|
||||
agent_provider_template="claude"):
|
||||
from bot_bottle.manifest import Manifest, ManifestBottle
|
||||
from bot_bottle.manifest_agent import ManifestAgent, ManifestAgentProvider
|
||||
from bot_bottle.manifest_egress import ManifestEgressConfig
|
||||
from bot_bottle.manifest_git import ManifestGitUser
|
||||
|
||||
agent = ManifestAgent(skills=tuple(skills))
|
||||
bottle = ManifestBottle(
|
||||
env=env or {},
|
||||
supervise=supervise,
|
||||
agent_provider=ManifestAgentProvider(template=agent_provider_template),
|
||||
)
|
||||
return Manifest(agent=agent, bottle=bottle)
|
||||
|
||||
def test_includes_agent_section(self):
|
||||
m = self._make_manifest_obj(skills=["researcher"])
|
||||
yaml = start_mod._manifest_to_yaml(m)
|
||||
self.assertIn("agent:", yaml)
|
||||
self.assertIn("- researcher", yaml)
|
||||
|
||||
def test_includes_bottle_section(self):
|
||||
m = self._make_manifest_obj(env={"FOO": "bar"})
|
||||
yaml = start_mod._manifest_to_yaml(m)
|
||||
self.assertIn("bottle:", yaml)
|
||||
self.assertIn("FOO: bar", yaml)
|
||||
|
||||
def test_supervise_rendered(self):
|
||||
m_true = self._make_manifest_obj(supervise=True)
|
||||
m_false = self._make_manifest_obj(supervise=False)
|
||||
self.assertIn("supervise: true", start_mod._manifest_to_yaml(m_true))
|
||||
self.assertIn("supervise: false", start_mod._manifest_to_yaml(m_false))
|
||||
|
||||
def test_non_claude_provider_shown(self):
|
||||
m = self._make_manifest_obj(agent_provider_template="codex")
|
||||
yaml = start_mod._manifest_to_yaml(m)
|
||||
self.assertIn("agent_provider:", yaml)
|
||||
self.assertIn("template: codex", yaml)
|
||||
|
||||
def test_default_claude_provider_omitted(self):
|
||||
m = self._make_manifest_obj(agent_provider_template="claude")
|
||||
yaml = start_mod._manifest_to_yaml(m)
|
||||
self.assertNotIn("agent_provider:", yaml)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -12,6 +12,9 @@ from typing import Any, Optional
|
||||
|
||||
from bot_bottle.cli.tui import _filter_items, _multiselect_loop, filter_multiselect, filter_select
|
||||
|
||||
_KEY_SPACE = 32
|
||||
_KEY_ENTER = 10
|
||||
|
||||
_KEY_ESC = 27
|
||||
_KEY_CTRL_D = 4
|
||||
|
||||
@@ -144,7 +147,29 @@ class TestMultiselectLoopReordering(unittest.TestCase):
|
||||
)
|
||||
self.assertEqual(["a", "b"], result)
|
||||
|
||||
def test_space_toggles_item_on(self):
|
||||
# Space on an unselected item selects it; Ctrl-D confirms.
|
||||
result = self._run([_KEY_SPACE, _KEY_CTRL_D], ["a", "b"], [])
|
||||
self.assertEqual(["a"], result)
|
||||
|
||||
def test_space_toggles_item_off(self):
|
||||
# Space on a selected item deselects it; Ctrl-D confirms empty.
|
||||
result = self._run([_KEY_SPACE, _KEY_CTRL_D], ["a", "b"], ["a"])
|
||||
self.assertEqual([], result)
|
||||
|
||||
def test_enter_confirms_without_toggle(self):
|
||||
# Enter immediately confirms the current selection without toggling.
|
||||
result = self._run([_KEY_ENTER], ["a", "b"], ["a"])
|
||||
self.assertEqual(["a"], result)
|
||||
|
||||
def test_enter_confirms_empty_selection(self):
|
||||
result = self._run([_KEY_ENTER], ["a", "b"], [])
|
||||
self.assertEqual([], result)
|
||||
|
||||
def test_space_then_enter_confirms(self):
|
||||
# Space selects "a", Enter confirms.
|
||||
result = self._run([_KEY_SPACE, _KEY_ENTER], ["a", "b"], [])
|
||||
self.assertEqual(["a"], result)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
Reference in New Issue
Block a user