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
@@ -33,7 +33,7 @@ from ..bottle_state import (
|
|||||||
)
|
)
|
||||||
# from ..backend.docker.capability_apply import snapshot_transcript
|
# from ..backend.docker.capability_apply import snapshot_transcript
|
||||||
from ..log import info
|
from ..log import info
|
||||||
from ..manifest import ManifestIndex
|
from ..manifest import Manifest, ManifestIndex
|
||||||
from ._common import PROG, USER_CWD, read_tty_line
|
from ._common import PROG, USER_CWD, read_tty_line
|
||||||
from . import tui
|
from . import tui
|
||||||
|
|
||||||
@@ -77,16 +77,19 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
# Bottle multiselect: always show after agent selection so operators
|
# Bottle multiselect: always show after agent selection so operators
|
||||||
# can compose bottles at launch time without editing agent manifests.
|
# can compose bottles at launch time without editing agent manifests.
|
||||||
available_bottles = manifest.all_bottle_names
|
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_bottle = _peek_agent_bottle(manifest, agent_name)
|
||||||
initial_bottles = [initial_bottle] if initial_bottle else []
|
initial_labels = [lineage_map.get(initial_bottle, initial_bottle)] if initial_bottle else []
|
||||||
selected_bottles = tui.filter_multiselect(
|
selected_labels = tui.filter_multiselect(
|
||||||
available_bottles,
|
display_labels,
|
||||||
title="Select bottles",
|
title="Select bottles",
|
||||||
initial=initial_bottles,
|
initial=initial_labels,
|
||||||
)
|
)
|
||||||
if selected_bottles is None:
|
if selected_labels is None:
|
||||||
return 0
|
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 = tui.name_color_modal(default_label=agent_name)
|
||||||
label, color = _resolve_unique_label(label, color)
|
label, color = _resolve_unique_label(label, color)
|
||||||
@@ -263,10 +266,112 @@ def _text_prompt_yes() -> bool:
|
|||||||
|
|
||||||
def _text_render_preflight():
|
def _text_render_preflight():
|
||||||
def _render(plan: DockerBottlePlan) -> None:
|
def _render(plan: DockerBottlePlan) -> None:
|
||||||
plan.print()
|
print(file=sys.stderr)
|
||||||
|
print(_manifest_to_yaml(plan.manifest), file=sys.stderr)
|
||||||
return _render
|
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(
|
def _launch_bottle(
|
||||||
spec: BottleSpec,
|
spec: BottleSpec,
|
||||||
*,
|
*,
|
||||||
|
|||||||
@@ -29,7 +29,8 @@ def filter_multiselect(
|
|||||||
Returns the ordered list of selected items, or ``None`` if the user
|
Returns the ordered list of selected items, or ``None`` if the user
|
||||||
cancelled (Esc / ``q`` / Ctrl-C / Ctrl-D with no items).
|
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 Ctrl-D to confirm the current selection (returns even if empty).
|
||||||
Press Esc/q to cancel (returns None).
|
Press Esc/q to cancel (returns None).
|
||||||
|
|
||||||
@@ -356,7 +357,10 @@ def _multiselect_loop(
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
if focus == "filter":
|
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:
|
if filtered:
|
||||||
item = filtered[cursor]
|
item = filtered[cursor]
|
||||||
if item in selected:
|
if item in selected:
|
||||||
@@ -500,7 +504,7 @@ def _render_multiselect(
|
|||||||
row += 1
|
row += 1
|
||||||
|
|
||||||
if focus == "filter":
|
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:
|
else:
|
||||||
help_line = "[↑↓/jk] cursor [K/J] reorder [Space/Enter] remove [Tab] back [Ctrl-D] done"
|
help_line = "[↑↓/jk] cursor [K/J] reorder [Space/Enter] remove [Tab] back [Ctrl-D] done"
|
||||||
if row < rows:
|
if row < rows:
|
||||||
|
|||||||
@@ -253,5 +253,103 @@ class TestCmdStartLabelCollision(unittest.TestCase):
|
|||||||
self.assertIn("already in use", second_call_kwargs.get("disclaimer", ""))
|
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__":
|
if __name__ == "__main__":
|
||||||
unittest.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
|
from bot_bottle.cli.tui import _filter_items, _multiselect_loop, filter_multiselect, filter_select
|
||||||
|
|
||||||
|
_KEY_SPACE = 32
|
||||||
|
_KEY_ENTER = 10
|
||||||
|
|
||||||
_KEY_ESC = 27
|
_KEY_ESC = 27
|
||||||
_KEY_CTRL_D = 4
|
_KEY_CTRL_D = 4
|
||||||
|
|
||||||
@@ -144,7 +147,29 @@ class TestMultiselectLoopReordering(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertEqual(["a", "b"], result)
|
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__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user