From df3d6d8f72e2dff759377ba84a6fcc81dc6325bb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 19:24:59 +0000 Subject: [PATCH 1/2] Dashboard: pick a per-task agent CLI in the `n` new-task flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-task `agent_cli` override has had a full data path since #384/#385 (model, store, service, REST, client, MCP) but no way to set it interactively — the dashboard's `n` flow went repo -> workflow -> memo and always inherited the repo default. Insert a CLI step between workflow and memo, pre-selected to the repo's own default so the common path is still Enter/Enter/Enter-and-type. Picking the default sends `agent_cli=None` rather than a redundant explicit value, matching `resolve_agent_cli`'s contract. - `KNOWN_AGENT_CLIS` in `core/models.py`. The CLI name set is a control-plane concept (it drives the base-image variant + config mount), and `terminal`/`taskservice` can't import `container/cli` at runtime without breaking the determinism invariant, so the list is restated in the LLM-free core and guarded by a test. - `registered_agent_clis()` on the adapter registry — the registry is populated lazily, so a drift test comparing against `_REGISTRY` directly would pass vacuously. The new guard also pins the two long-duplicated `DEFAULT_AGENT_CLI` constants together. - `_OptionListModal` grows an `initial=` param; it had no way to set the starting highlight, which the pre-select needs. The repo default is named in the picker title rather than baked into an option label, since `ChoiceScreen` dismisses the option text. - Read the repo default from the `list_repos()` payload `action_new_task` already fetches, not `self._repo_clis` — that cache is only filled on a refresh pass and can be empty or stale at this point. - Swap four hardcoded `"claude"` literals in `dashboard.py` for `DEFAULT_AGENT_CLI`. The fake-adapter registration test now cleans up after itself; it was leaking `fake-cli` into the module-global registry, which would have made the drift guard order-dependent. Co-Authored-By: Claude --- src/panopticon/container/cli/__init__.py | 9 +- src/panopticon/container/cli/base.py | 11 +++ src/panopticon/core/models.py | 9 ++ src/panopticon/terminal/dashboard.py | 108 ++++++++++++++++------- tests/container/test_cli_base.py | 23 ++++- tests/terminal/test_dashboard.py | 108 ++++++++++++++++++++++- 6 files changed, 230 insertions(+), 38 deletions(-) diff --git a/src/panopticon/container/cli/__init__.py b/src/panopticon/container/cli/__init__.py index bf6b6215..7c910273 100644 --- a/src/panopticon/container/cli/__init__.py +++ b/src/panopticon/container/cli/__init__.py @@ -8,6 +8,13 @@ AgentCLI, get_agent_cli, register_agent_cli, + registered_agent_clis, ) -__all__ = ["DEFAULT_AGENT_CLI", "AgentCLI", "get_agent_cli", "register_agent_cli"] +__all__ = [ + "DEFAULT_AGENT_CLI", + "AgentCLI", + "get_agent_cli", + "register_agent_cli", + "registered_agent_clis", +] diff --git a/src/panopticon/container/cli/base.py b/src/panopticon/container/cli/base.py index 97263286..33bcb231 100644 --- a/src/panopticon/container/cli/base.py +++ b/src/panopticon/container/cli/base.py @@ -172,6 +172,17 @@ def get_agent_cli(name: str | None = None) -> AgentCLI: raise KeyError(f"unknown agent CLI {key!r}; registered: {sorted(_REGISTRY)}") from None +def registered_agent_clis() -> tuple[str, ...]: + """Return the registered adapter names, sorted (loading the built-ins first). + + The registry is populated lazily, so callers that only want the *names* need this rather than + reaching into :data:`_REGISTRY` (which is empty until an adapter module is imported). Used by + the drift test that keeps :data:`panopticon.core.models.KNOWN_AGENT_CLIS` honest. + """ + _load_builtin_adapters() + return tuple(sorted(_REGISTRY)) + + def _load_builtin_adapters() -> None: """Register the built-in adapters (imported lazily so this module holds only the contract).""" from panopticon.container.cli.claude import ClaudeAgentCLI diff --git a/src/panopticon/core/models.py b/src/panopticon/core/models.py index 610adf98..847b5c21 100644 --- a/src/panopticon/core/models.py +++ b/src/panopticon/core/models.py @@ -226,6 +226,15 @@ class Repo: #: The CLI a task falls back to when neither the task nor its repo names one (ADR 0014 §2/§3). DEFAULT_AGENT_CLI = "claude" +#: The CLI names an operator can pick from (the dashboard's new-task picker offers these). +#: +#: The set of CLIs is a **control-plane** concept — it drives the base-image variant and the +#: config-dir mount — so it lives here, in the LLM-free core, rather than being read from the +#: adapter registry in :mod:`panopticon.container.cli` (the only LLM-bearing package; importing it +#: from ``terminal``/``taskservice`` would break the determinism invariant). The two are kept in +#: sync by a test — see ``tests/container/test_cli_base.py``. +KNOWN_AGENT_CLIS: tuple[str, ...] = ("claude", "codex") + def resolve_agent_cli(task_agent_cli: str | None, repo_agent_cli: str | None) -> str: """Resolve a task's effective agent CLI: its own override → the repo default → ``"claude"``. diff --git a/src/panopticon/terminal/dashboard.py b/src/panopticon/terminal/dashboard.py index fa740695..a599430e 100644 --- a/src/panopticon/terminal/dashboard.py +++ b/src/panopticon/terminal/dashboard.py @@ -105,7 +105,7 @@ from panopticon.client import JsonObj, TaskServiceClient from panopticon.core.artifacts import InvalidArtifactName, validate_segment from panopticon.core.dirs import ARTIFACTS_DIR -from panopticon.core.models import resolve_agent_cli +from panopticon.core.models import DEFAULT_AGENT_CLI, KNOWN_AGENT_CLIS, resolve_agent_cli from panopticon.core.state import TERMINAL_LABELS from panopticon.sessionservice.local_runner import session_name from panopticon.taskservice.artifacts_fs import FilesystemArtifactStore @@ -624,10 +624,13 @@ class _OptionListModal(ModalScreen[_ResultT | None]): BINDINGS = [("escape", "cancel", "Cancel")] BOX_ID = "list-box" - def __init__(self, title: str, options: list[str]) -> None: + def __init__(self, title: str, options: list[str], *, initial: str | None = None) -> None: super().__init__() self._title = title self._options = options + #: Which option starts highlighted, so Enter alone picks the expected one. Ignored when it + #: isn't in ``options``. + self._initial = initial def compose(self) -> ComposeResult: with Vertical(id=self.BOX_ID): @@ -639,7 +642,10 @@ def _extra_widgets(self) -> Iterable[Widget]: return () def on_mount(self) -> None: - self.query_one(OptionList).focus() + option_list = self.query_one(OptionList) + if self._initial is not None and self._initial in self._options: + option_list.highlighted = self._options.index(self._initial) + option_list.focus() def action_cancel(self) -> None: self.dismiss(None) @@ -1407,13 +1413,13 @@ def __init__( def _initial(self, name: str) -> str: """A field's pre-populated value: the repo's stored value, else (create mode only) - ``main`` for ``default_base`` / ``claude`` for ``agent_cli``, else blank.""" + ``main`` for ``default_base`` / the default CLI for ``agent_cli``, else blank.""" stored = self._repo.get(name) if stored: return str(stored) if self._editing: return "" - return {"default_base": "main", "agent_cli": "claude"}.get(name, "") + return {"default_base": "main", "agent_cli": DEFAULT_AGENT_CLI}.get(name, "") def _wf_checked(self, wf: dict[str, Any]) -> bool: name = wf["name"] @@ -1617,7 +1623,7 @@ def create(values: dict[str, Any]) -> str | None: capabilities={"docker_in_docker": values["docker_in_docker"]}, enabled_workflows=values["enabled_workflows"], disabled_workflows=values["disabled_workflows"], - agent_cli=values["agent_cli"] or "claude", + agent_cli=values["agent_cli"] or DEFAULT_AGENT_CLI, ) except httpx.HTTPStatusError as exc: return f"Can't create: {_detail(exc)}" @@ -1653,7 +1659,7 @@ def save(values: dict[str, Any]) -> str | None: capabilities=capabilities, enabled_workflows=values["enabled_workflows"], disabled_workflows=values["disabled_workflows"], - agent_cli=values["agent_cli"] or "claude", + agent_cli=values["agent_cli"] or DEFAULT_AGENT_CLI, ) except httpx.HTTPStatusError as exc: return f"Can't update: {_detail(exc)}" @@ -1985,7 +1991,9 @@ def _load_repo_names(self) -> None: try: repos = self._client.list_repos() self._repo_names = {str(r["id"]): str(r["name"]) for r in repos} - self._repo_clis = {str(r["id"]): str(r.get("agent_cli") or "claude") for r in repos} + self._repo_clis = { + str(r["id"]): str(r.get("agent_cli") or DEFAULT_AGENT_CLI) for r in repos + } except Exception: pass @@ -2242,11 +2250,20 @@ def on_data_table_row_selected(self, event: DataTable.RowSelected) -> None: self.action_refresh() def action_new_task(self) -> None: - """`n`: create a task — pick a repo, a workflow, describe the work, then POST it.""" - repos = [str(r["id"]) for r in self._client.list_repos()] - if not repos: + """`n`: create a task — pick a repo, a workflow, an agent CLI, describe the work, then POST it. + + The CLI step is pre-selected to the repo's own default, so the common path stays + Enter/Enter/Enter-and-type; picking that default sends no override at all (ADR 0014 §3).""" + repo_records = self._client.list_repos() + if not repo_records: self.notify("Need at least one repo to create a task.", severity="warning") return + repos = [str(r["id"]) for r in repo_records] + # Read each repo's CLI from the payload we just fetched rather than `self._repo_clis`, which + # is only filled in on a refresh pass and so can be empty or stale here. + repo_clis = { + str(r["id"]): str(r.get("agent_cli") or DEFAULT_AGENT_CLI) for r in repo_records + } def pick_workflow(repo: str | None) -> None: if repo is None: @@ -2256,34 +2273,57 @@ def pick_workflow(repo: str | None) -> None: self.notify(f"No workflows enabled for repo {repo!r}.", severity="warning") return - def describe(workflow: str | None) -> None: + def pick_cli(workflow: str | None) -> None: if workflow is None: return + repo_default = repo_clis.get(repo, DEFAULT_AGENT_CLI) - def create(result: tuple[str, bool, dict[str, str]] | None) -> None: - if result is None: # backed out + def describe(cli: str | None) -> None: + if cli is None: return - memo_text, submit, artifacts_b64 = result - stripped = memo_text.strip() - if _apply_memo_filter(stripped): - return - if submit and stripped: - self._client.create_task( - repo, - workflow, - stripped, - initial_prompt=stripped, - artifacts_b64=artifacts_b64, - ) - else: - self._client.create_task( - repo, workflow, stripped or None, artifacts_b64=artifacts_b64 - ) - self.action_refresh() - - self.push_screen(MemoScreen(), create) + # None = "no override, use the repo default" — what `resolve_agent_cli` expects. + agent_cli = None if cli == repo_default else cli + + def create(result: tuple[str, bool, dict[str, str]] | None) -> None: + if result is None: # backed out + return + memo_text, submit, artifacts_b64 = result + stripped = memo_text.strip() + if _apply_memo_filter(stripped): + return + if submit and stripped: + self._client.create_task( + repo, + workflow, + stripped, + initial_prompt=stripped, + artifacts_b64=artifacts_b64, + agent_cli=agent_cli, + ) + else: + self._client.create_task( + repo, + workflow, + stripped or None, + artifacts_b64=artifacts_b64, + agent_cli=agent_cli, + ) + self.action_refresh() + + self.push_screen(MemoScreen(), create) + + # The default is named in the title, not baked into an option label — ChoiceScreen + # dismisses the option's text, so a decorated label would need stripping back off. + self.push_screen( + ChoiceScreen( + f"agent CLI (repo default: {repo_default})", + list(KNOWN_AGENT_CLIS), + initial=repo_default, + ), + describe, + ) - self.push_screen(WorkflowScreen(workflows), describe) + self.push_screen(WorkflowScreen(workflows), pick_cli) self.push_screen(ChoiceScreen("repo", repos), pick_workflow) diff --git a/tests/container/test_cli_base.py b/tests/container/test_cli_base.py index 1fdf7eab..a759c192 100644 --- a/tests/container/test_cli_base.py +++ b/tests/container/test_cli_base.py @@ -13,11 +13,14 @@ from panopticon.container.cli import ( DEFAULT_AGENT_CLI, AgentCLI, + base, get_agent_cli, register_agent_cli, + registered_agent_clis, ) from panopticon.container.cli.claude import ClaudeAgentCLI from panopticon.container.cli.codex import CodexAgentCLI +from panopticon.core import models as core_models def test_default_resolves_to_claude() -> None: @@ -26,6 +29,17 @@ def test_default_resolves_to_claude() -> None: assert isinstance(get_agent_cli("claude"), ClaudeAgentCLI) +def test_core_agent_cli_constants_match_the_adapter_registry() -> None: + """The control plane can't import this package at runtime (the determinism invariant — `core` + stays LLM-free), so `core.models` re-states the CLI name list and the default. This is the guard + that keeps the two copies from drifting: add an adapter, and it fails until `core` knows about it. + """ + assert list(registered_agent_clis()) == sorted(core_models.KNOWN_AGENT_CLIS) + assert core_models.DEFAULT_AGENT_CLI == DEFAULT_AGENT_CLI + # The fallback must itself be a selectable CLI. + assert core_models.DEFAULT_AGENT_CLI in core_models.KNOWN_AGENT_CLIS + + def test_codex_is_a_registered_built_in_adapter() -> None: # The second built-in CLI: registering it makes it resolvable with no launcher edit (ADR 0014 §2). assert isinstance(get_agent_cli("codex"), CodexAgentCLI) @@ -73,8 +87,13 @@ def launch(self, config_dir: Path) -> None: # pragma: no cover - not exercised pass register_agent_cli(_Fake) - resolved = get_agent_cli("fake-cli") - assert isinstance(resolved, _Fake) and resolved.config_dirname == ".fake" + try: + resolved = get_agent_cli("fake-cli") + assert isinstance(resolved, _Fake) and resolved.config_dirname == ".fake" + finally: + # The registry is module-global: leaving the fake in it would leak into any other test that + # inspects the registered names (e.g. the KNOWN_AGENT_CLIS drift guard above). + base._REGISTRY.pop("fake-cli", None) # -- shared base-class behaviour ---------------------------------------------------------------- diff --git a/tests/terminal/test_dashboard.py b/tests/terminal/test_dashboard.py index 8b841749..d1a36b24 100644 --- a/tests/terminal/test_dashboard.py +++ b/tests/terminal/test_dashboard.py @@ -15,13 +15,15 @@ import httpx import pytest from textual.app import App -from textual.widgets import Checkbox, DataTable, Input, Select, Static +from textual.widgets import Checkbox, DataTable, Input, OptionList, Select, Static +from panopticon.core.models import KNOWN_AGENT_CLIS from panopticon.terminal import dashboard from panopticon.terminal.dashboard import ( _ENSEMBLE_KEY_PREFIX, _INDEFINITE_SNOOZE_UNTIL, _SNOOZE_DURATION, + ChoiceScreen, Dashboard, SpaceCheckbox, TaskDetailScreen, @@ -115,6 +117,7 @@ def __init__( self.list_tasks_calls = 0 # how many times the table was (re)built — counts feed refreshes self.created: list[tuple[str, str, str | None]] = [] self.created_artifacts_b64: list[dict[str, str] | None] = [] + self.created_agent_clis: list[str | None] = [] self.applied: list[tuple[str, str]] = [] self.released: list[str] = [] self.snoozed: list[tuple[str, str | None]] = [] @@ -236,9 +239,11 @@ def create_task( initial_prompt: str | None = None, artifacts: dict[str, str] | None = None, artifacts_b64: dict[str, str] | None = None, + agent_cli: str | None = None, ) -> dict[str, Any]: self.created.append((repo_id, workflow, memo, initial_prompt)) self.created_artifacts_b64.append(artifacts_b64) + self.created_agent_clis.append(agent_cli) return {"id": "new"} def apply_operation(self, task_id: str, operation: str) -> dict[str, Any]: @@ -810,11 +815,90 @@ async def test_pressing_n_creates_a_task_via_repo_workflow_then_memo() -> None: await pilot.pause() await pilot.press("enter") # first (only) workflow: spike await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("f", "i", "x") # type a memo into the prompt await pilot.press("enter") # submit await pilot.pause() # Enter always submits the memo as the agent's initial prompt assert fake.created == [("r1", "spike", "fix", "fix")] + # The CLI step was left on the repo default → no per-task override recorded. + assert fake.created_agent_clis == [None] + + +async def test_pressing_n_taking_the_repo_default_cli_sends_no_override() -> None: + # A repo whose default is codex: accepting the pre-selected entry must still record *no* + # override, not a redundant agent_cli="codex" (resolve_agent_cli's contract — ADR 0014 §3). + fake = _FakeClient( + [], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "codex"}], + workflows=[{"name": "spike", "when_to_use": ""}], + ) + app = Dashboard(fake) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("n") + await pilot.pause() + await pilot.press("enter") # repo + await pilot.pause() + await pilot.press("enter") # workflow + await pilot.pause() + await pilot.press("enter") # agent CLI: codex is pre-selected as the repo default + await pilot.pause() + await pilot.press("f", "i", "x") + await pilot.press("enter") + await pilot.pause() + assert fake.created == [("r1", "spike", "fix", "fix")] + assert fake.created_agent_clis == [None] + + +async def test_pressing_n_can_override_the_repo_default_cli() -> None: + # The repo defaults to claude; move off the pre-selected entry to pick codex for this one task. + fake = _FakeClient( + [], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "claude"}], + workflows=[{"name": "spike", "when_to_use": ""}], + ) + app = Dashboard(fake) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("n") + await pilot.pause() + await pilot.press("enter") # repo + await pilot.pause() + await pilot.press("enter") # workflow + await pilot.pause() + await pilot.press("j") # off the claude default… + await pilot.press("enter") # …onto codex + await pilot.pause() + await pilot.press("f", "i", "x") + await pilot.press("enter") + await pilot.pause() + assert fake.created == [("r1", "spike", "fix", "fix")] + assert fake.created_agent_clis == ["codex"] + + +async def test_new_task_cli_picker_preselects_the_repo_default() -> None: + # The repo default starts highlighted, so Enter alone keeps it — the fast path stays fast even + # when the default isn't first in KNOWN_AGENT_CLIS. + fake = _FakeClient( + [], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "codex"}], + workflows=[{"name": "spike", "when_to_use": ""}], + ) + app = Dashboard(fake) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("n") + await pilot.pause() + await pilot.press("enter") # repo + await pilot.pause() + await pilot.press("enter") # workflow + await pilot.pause() + screen = app.screen + assert isinstance(screen, ChoiceScreen) + option_list = screen.query_one(OptionList) + assert option_list.highlighted == list(KNOWN_AGENT_CLIS).index("codex") async def test_pressing_n_with_a_blank_memo_creates_with_none() -> None: @@ -832,6 +916,8 @@ async def test_pressing_n_with_a_blank_memo_creates_with_none() -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("enter") # submit an empty memo await pilot.pause() assert fake.created == [("r1", "spike", None, None)] @@ -853,6 +939,8 @@ async def test_memo_ctrl_s_sets_the_memo_without_submitting() -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("f", "i", "x") # type a memo await pilot.press("ctrl+s") # set without submitting await pilot.pause() @@ -880,6 +968,8 @@ async def test_memo_ctrl_g_opens_editor_and_updates_textarea(monkeypatch: Any) - await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("h", "i") # type initial text await pilot.press("ctrl+g") # open editor await pilot.pause() @@ -907,6 +997,8 @@ async def test_memo_textarea_expands_for_multiline_content(monkeypatch: Any) -> await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("ctrl+g") await pilot.pause() await pilot.press("enter") # submit @@ -932,6 +1024,8 @@ async def test_memo_ctrl_a_attaches_a_file_as_an_artifact(tmp_path: Path) -> Non await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("f", "i", "x") # type a memo await pilot.press("ctrl+a") # open the attach-files modal await pilot.pause() @@ -972,6 +1066,8 @@ async def test_memo_ctrl_a_preserves_spaces_in_the_filename(tmp_path: Path) -> N await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -1007,6 +1103,8 @@ async def test_memo_ctrl_a_attaches_a_binary_file(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -1040,6 +1138,8 @@ async def test_memo_ctrl_a_can_remove_a_queued_file(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("ctrl+a") # open the attach-files modal await pilot.pause() screen = app.screen @@ -1078,6 +1178,8 @@ async def test_memo_ctrl_a_rejects_a_missing_path(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -1135,6 +1237,8 @@ async def test_memo_ctrl_a_accepts_a_quoted_path(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -3471,6 +3575,8 @@ async def test_pressing_j_then_enter_picks_the_second_option_in_a_picker() -> No await pilot.pause() await pilot.press("enter") # workflow: spike (only one) await pilot.pause() + await pilot.press("enter") # agent CLI: the repo default, pre-selected + await pilot.pause() await pilot.press("enter") # submit an empty memo await pilot.pause() assert fake.created == [("r2", "spike", None, None)] From 03b78b2d5cf47e6e56d3cbfbf9dd4f3e531dd141 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 19:45:52 +0000 Subject: [PATCH 2/2] Move the CLI picker into the memo modal as a dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The separate ChoiceScreen step made task creation four screens deep and cost a keystroke even when the operator didn't care which CLI ran. Fold it into the memo modal as a labelled Select that starts on the repo's default, so the common path is back to repo -> workflow -> type -> enter and the CLI is right there when wanted. - MemoScreen takes `agent_clis` + `repo_default` and dismisses a 4-tuple, the added member being the picked CLI. Both default to empty, so the screen still stands alone with no dropdown. - Tab reaches the dropdown (TextArea's default tab_behavior is "focus"); picking hands focus back to the memo, so the next Enter submits rather than reopening the overlay. The refocus is deliberately unconditional — while the overlay is open, focus is on the overlay rather than the Select, so a has_focus guard never fires on the path that matters. - `_selected_cli` falls back to the first known CLI when a repo names one this build doesn't offer, since allow_blank=False needs a value that's really in the options. - Reverts the `initial=` param added to _OptionListModal last commit; with no CLI ChoiceScreen there is nothing left that pre-selects. The 12 flow tests lose the extra `enter` they grew last commit. New coverage for the dropdown: repo default shown and sent as no-override, an explicit override, every known CLI on offer, an unknown one rejected, an unofferable repo default, and the full tab -> pick -> focus-returns interaction (which caught the focus bug above). Co-Authored-By: Claude --- src/panopticon/terminal/dashboard.py | 143 +++++++++++++++----------- tests/terminal/test_dashboard.py | 144 +++++++++++++++++++-------- 2 files changed, 184 insertions(+), 103 deletions(-) diff --git a/src/panopticon/terminal/dashboard.py b/src/panopticon/terminal/dashboard.py index a599430e..b9883f31 100644 --- a/src/panopticon/terminal/dashboard.py +++ b/src/panopticon/terminal/dashboard.py @@ -68,7 +68,7 @@ import tempfile import time import webbrowser -from collections.abc import Callable, Iterable +from collections.abc import Callable, Iterable, Sequence from dataclasses import dataclass from datetime import UTC, datetime, timedelta from math import ceil @@ -624,13 +624,10 @@ class _OptionListModal(ModalScreen[_ResultT | None]): BINDINGS = [("escape", "cancel", "Cancel")] BOX_ID = "list-box" - def __init__(self, title: str, options: list[str], *, initial: str | None = None) -> None: + def __init__(self, title: str, options: list[str]) -> None: super().__init__() self._title = title self._options = options - #: Which option starts highlighted, so Enter alone picks the expected one. Ignored when it - #: isn't in ``options``. - self._initial = initial def compose(self) -> ComposeResult: with Vertical(id=self.BOX_ID): @@ -642,10 +639,7 @@ def _extra_widgets(self) -> Iterable[Widget]: return () def on_mount(self) -> None: - option_list = self.query_one(OptionList) - if self._initial is not None and self._initial in self._options: - option_list.highlighted = self._options.index(self._initial) - option_list.focus() + self.query_one(OptionList).focus() def action_cancel(self) -> None: self.dismiss(None) @@ -723,16 +717,21 @@ def on_text_area_changed(self, event: TextArea.Changed) -> None: self.styles.height = min(lines, self.MAX_LINES) -class MemoScreen(ModalScreen["tuple[str, bool, dict[str, str]] | None"]): +class MemoScreen(ModalScreen["tuple[str, bool, dict[str, str], str] | None"]): """Memo prompt for task creation. - Dismisses ``(text, submit, artifacts_b64)`` where ``submit`` says whether to deliver the memo as - the agent's initial prompt and ``artifacts_b64`` is a ``name → base64`` map of files attached via - ``ctrl+a`` (base64 so binary files like screenshots seed intact), or ``None`` on cancel + Dismisses ``(text, submit, artifacts_b64, agent_cli)`` where ``submit`` says whether to deliver + the memo as the agent's initial prompt, ``artifacts_b64`` is a ``name → base64`` map of files + attached via ``ctrl+a`` (base64 so binary files like screenshots seed intact), and ``agent_cli`` + is the CLI picked from the dropdown (which starts on the repo's default) — or ``None`` on cancel (Escape). **Enter always submits** the memo as an initial prompt; **ctrl+s sets the memo without submitting** it (an unsent paste); **ctrl+a** opens the attach-files modal (:class:`ArtifactsScreen`). + The CLI lives here rather than in a picker modal of its own so the common path is a single + screen: the dropdown already shows the repo default, so an operator who doesn't care never has + to touch it (ADR 0014 §3). + Uses :class:`MemoTextArea` so Enter submits rather than inserting a newline — same UX as the original single-line ``Input``, but the field can display multi-line content loaded by ``ctrl+g`` (open in ``$EDITOR``).""" @@ -741,6 +740,7 @@ class MemoScreen(ModalScreen["tuple[str, bool, dict[str, str]] | None"]): MemoScreen { align: center middle; } #memo-box { width: 64; height: auto; padding: 1 2; border: round $accent; background: $surface; } #memo-box MemoTextArea { height: 1; margin-bottom: 1; } + #memo-box #memo-cli-select { margin-bottom: 1; } #memo-box .memo-hint { color: $text-muted; } #memo-attached { color: $text-muted; } """ @@ -752,17 +752,30 @@ class MemoScreen(ModalScreen["tuple[str, bool, dict[str, str]] | None"]): ("enter", "submit", "Create"), ] - def __init__(self) -> None: + def __init__(self, *, agent_clis: Sequence[str] = (), repo_default: str = "") -> None: super().__init__() # name → (source path, raw bytes) for the files the operator attaches via ctrl+a. Keyed by # the artifact name (the path's basename) so a re-attach of the same name overwrites. Bytes, # not text, so binary files (screenshots, PDFs) attach intact. self._artifacts: dict[str, tuple[str, bytes]] = {} + # The CLIs on offer and the one the dropdown starts on (the repo's own default). Defaulting + # both to empty keeps the screen usable on its own — with no options there's no dropdown. + self._agent_clis = list(agent_clis) + self._repo_default = repo_default def compose(self) -> ComposeResult: with Vertical(id="memo-box"): yield MemoTextArea(compact=True) yield Label("", id="memo-attached") + if self._agent_clis: + # Labelled, because a bare "codex" in a box doesn't say what it selects. + yield Label("agent CLI (tab to change)", classes="memo-hint") + yield Select( + [(cli, cli) for cli in self._agent_clis], + value=self._selected_cli(), + allow_blank=False, + id="memo-cli-select", + ) yield Label("enter: submit", classes="memo-hint") yield Label("ctrl+s: set without submitting", classes="memo-hint") yield Label("ctrl+g: edit in $EDITOR", classes="memo-hint") @@ -772,6 +785,29 @@ def on_mount(self) -> None: self.query_one(MemoTextArea).focus() self._refresh_attached() + def _selected_cli(self) -> str: + """The CLI the dropdown is on — the repo default until the operator changes it. + + Falls back to the first offered CLI if the repo names one we don't offer, so the value is + always a real option (``Select`` with ``allow_blank=False`` requires one).""" + if self._repo_default in self._agent_clis: + return self._repo_default + return self._agent_clis[0] if self._agent_clis else "" + + def _picked_cli(self) -> str: + """The dropdown's current value, or the repo default when there's no dropdown.""" + if not self._agent_clis: + return self._repo_default + return str(self.query_one("#memo-cli-select", Select).value) + + def on_select_changed(self, event: Select.Changed) -> None: + # Hand focus back to the memo so the next Enter submits rather than reopening the dropdown. + # Unconditional: while the overlay is open focus sits on *it*, not the Select, so a + # has_focus guard here would never fire on the path that matters. Harmless at mount time — + # Select posts an initial Changed, and on_mount focuses the memo anyway. + for memo in self.query(MemoTextArea): # empty until composed; a no-op then + memo.focus() + def _refresh_attached(self) -> None: """Update the "attached" line summarising the files the operator has queued.""" label = self.query_one("#memo-attached", Label) @@ -787,10 +823,12 @@ def _artifacts_b64(self) -> dict[str, str]: } def action_submit(self) -> None: - self.dismiss((self.query_one(MemoTextArea).text, True, self._artifacts_b64())) + text = self.query_one(MemoTextArea).text + self.dismiss((text, True, self._artifacts_b64(), self._picked_cli())) def action_set_only(self) -> None: - self.dismiss((self.query_one(MemoTextArea).text, False, self._artifacts_b64())) + text = self.query_one(MemoTextArea).text + self.dismiss((text, False, self._artifacts_b64(), self._picked_cli())) def action_cancel(self) -> None: self.dismiss(None) @@ -2250,10 +2288,10 @@ def on_data_table_row_selected(self, event: DataTable.RowSelected) -> None: self.action_refresh() def action_new_task(self) -> None: - """`n`: create a task — pick a repo, a workflow, an agent CLI, describe the work, then POST it. + """`n`: create a task — pick a repo, a workflow, describe the work, then POST it. - The CLI step is pre-selected to the repo's own default, so the common path stays - Enter/Enter/Enter-and-type; picking that default sends no override at all (ADR 0014 §3).""" + The agent CLI is a dropdown on the memo screen rather than a step of its own: it starts on + the repo's default, so leaving it alone sends no override at all (ADR 0014 §3).""" repo_records = self._client.list_repos() if not repo_records: self.notify("Need at least one repo to create a task.", severity="warning") @@ -2273,57 +2311,44 @@ def pick_workflow(repo: str | None) -> None: self.notify(f"No workflows enabled for repo {repo!r}.", severity="warning") return - def pick_cli(workflow: str | None) -> None: + def describe(workflow: str | None) -> None: if workflow is None: return repo_default = repo_clis.get(repo, DEFAULT_AGENT_CLI) - def describe(cli: str | None) -> None: - if cli is None: + def create(result: tuple[str, bool, dict[str, str], str] | None) -> None: + if result is None: # backed out + return + memo_text, submit, artifacts_b64, cli = result + stripped = memo_text.strip() + if _apply_memo_filter(stripped): return # None = "no override, use the repo default" — what `resolve_agent_cli` expects. agent_cli = None if cli == repo_default else cli + if submit and stripped: + self._client.create_task( + repo, + workflow, + stripped, + initial_prompt=stripped, + artifacts_b64=artifacts_b64, + agent_cli=agent_cli, + ) + else: + self._client.create_task( + repo, + workflow, + stripped or None, + artifacts_b64=artifacts_b64, + agent_cli=agent_cli, + ) + self.action_refresh() - def create(result: tuple[str, bool, dict[str, str]] | None) -> None: - if result is None: # backed out - return - memo_text, submit, artifacts_b64 = result - stripped = memo_text.strip() - if _apply_memo_filter(stripped): - return - if submit and stripped: - self._client.create_task( - repo, - workflow, - stripped, - initial_prompt=stripped, - artifacts_b64=artifacts_b64, - agent_cli=agent_cli, - ) - else: - self._client.create_task( - repo, - workflow, - stripped or None, - artifacts_b64=artifacts_b64, - agent_cli=agent_cli, - ) - self.action_refresh() - - self.push_screen(MemoScreen(), create) - - # The default is named in the title, not baked into an option label — ChoiceScreen - # dismisses the option's text, so a decorated label would need stripping back off. self.push_screen( - ChoiceScreen( - f"agent CLI (repo default: {repo_default})", - list(KNOWN_AGENT_CLIS), - initial=repo_default, - ), - describe, + MemoScreen(agent_clis=KNOWN_AGENT_CLIS, repo_default=repo_default), create ) - self.push_screen(WorkflowScreen(workflows), pick_cli) + self.push_screen(WorkflowScreen(workflows), describe) self.push_screen(ChoiceScreen("repo", repos), pick_workflow) diff --git a/tests/terminal/test_dashboard.py b/tests/terminal/test_dashboard.py index d1a36b24..b7e1d3af 100644 --- a/tests/terminal/test_dashboard.py +++ b/tests/terminal/test_dashboard.py @@ -15,7 +15,8 @@ import httpx import pytest from textual.app import App -from textual.widgets import Checkbox, DataTable, Input, OptionList, Select, Static +from textual.widgets import Checkbox, DataTable, Input, Select, Static +from textual.widgets._select import InvalidSelectValueError from panopticon.core.models import KNOWN_AGENT_CLIS from panopticon.terminal import dashboard @@ -23,8 +24,8 @@ _ENSEMBLE_KEY_PREFIX, _INDEFINITE_SNOOZE_UNTIL, _SNOOZE_DURATION, - ChoiceScreen, Dashboard, + MemoTextArea, SpaceCheckbox, TaskDetailScreen, _dim, @@ -815,8 +816,6 @@ async def test_pressing_n_creates_a_task_via_repo_workflow_then_memo() -> None: await pilot.pause() await pilot.press("enter") # first (only) workflow: spike await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("f", "i", "x") # type a memo into the prompt await pilot.press("enter") # submit await pilot.pause() @@ -826,9 +825,9 @@ async def test_pressing_n_creates_a_task_via_repo_workflow_then_memo() -> None: assert fake.created_agent_clis == [None] -async def test_pressing_n_taking_the_repo_default_cli_sends_no_override() -> None: - # A repo whose default is codex: accepting the pre-selected entry must still record *no* - # override, not a redundant agent_cli="codex" (resolve_agent_cli's contract — ADR 0014 §3). +async def test_memo_cli_dropdown_defaults_to_the_repo_cli_and_sends_no_override() -> None: + # A repo whose default is codex: leaving the dropdown alone must record *no* override, not a + # redundant agent_cli="codex" (resolve_agent_cli's contract — ADR 0014 §3). fake = _FakeClient( [], repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "codex"}], @@ -843,17 +842,16 @@ async def test_pressing_n_taking_the_repo_default_cli_sends_no_override() -> Non await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: codex is pre-selected as the repo default - await pilot.pause() - await pilot.press("f", "i", "x") + assert app.screen.query_one("#memo-cli-select", Select).value == "codex" + await pilot.press("f", "i", "x") # straight to typing — the dropdown needs no visit await pilot.press("enter") await pilot.pause() assert fake.created == [("r1", "spike", "fix", "fix")] assert fake.created_agent_clis == [None] -async def test_pressing_n_can_override_the_repo_default_cli() -> None: - # The repo defaults to claude; move off the pre-selected entry to pick codex for this one task. +async def test_memo_cli_dropdown_can_override_the_repo_default() -> None: + # The repo defaults to claude; change the dropdown to codex for this one task. fake = _FakeClient( [], repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "claude"}], @@ -868,8 +866,7 @@ async def test_pressing_n_can_override_the_repo_default_cli() -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("j") # off the claude default… - await pilot.press("enter") # …onto codex + app.screen.query_one("#memo-cli-select", Select).value = "codex" await pilot.pause() await pilot.press("f", "i", "x") await pilot.press("enter") @@ -878,12 +875,10 @@ async def test_pressing_n_can_override_the_repo_default_cli() -> None: assert fake.created_agent_clis == ["codex"] -async def test_new_task_cli_picker_preselects_the_repo_default() -> None: - # The repo default starts highlighted, so Enter alone keeps it — the fast path stays fast even - # when the default isn't first in KNOWN_AGENT_CLIS. +async def test_memo_cli_dropdown_offers_every_known_cli() -> None: fake = _FakeClient( [], - repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "codex"}], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "claude"}], workflows=[{"name": "spike", "when_to_use": ""}], ) app = Dashboard(fake) # type: ignore[arg-type] @@ -895,10 +890,57 @@ async def test_new_task_cli_picker_preselects_the_repo_default() -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - screen = app.screen - assert isinstance(screen, ChoiceScreen) - option_list = screen.query_one(OptionList) - assert option_list.highlighted == list(KNOWN_AGENT_CLIS).index("codex") + select = app.screen.query_one("#memo-cli-select", Select) + # Select validates against its own options, so assigning each name is the public way to + # assert every known CLI is on offer (an unknown one raises — see the test below). + for cli in KNOWN_AGENT_CLIS: + select.value = cli + assert select.value == cli + + +async def test_memo_cli_dropdown_rejects_a_cli_it_does_not_offer() -> None: + fake = _FakeClient( + [], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "claude"}], + workflows=[{"name": "spike", "when_to_use": ""}], + ) + app = Dashboard(fake) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("n") + await pilot.pause() + await pilot.press("enter") # repo + await pilot.pause() + await pilot.press("enter") # workflow + await pilot.pause() + select = app.screen.query_one("#memo-cli-select", Select) + with pytest.raises(InvalidSelectValueError): + select.value = "not-a-cli" + + +async def test_memo_cli_dropdown_survives_a_repo_cli_it_does_not_offer() -> None: + # A repo naming a CLI this dashboard build doesn't know (e.g. rolled back after an adapter was + # added): the dropdown must still hold a real option rather than blowing up on allow_blank=False. + fake = _FakeClient( + [], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "from-the-future"}], + workflows=[{"name": "spike", "when_to_use": ""}], + ) + app = Dashboard(fake) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("n") + await pilot.pause() + await pilot.press("enter") # repo + await pilot.pause() + await pilot.press("enter") # workflow + await pilot.pause() + assert app.screen.query_one("#memo-cli-select", Select).value == KNOWN_AGENT_CLIS[0] + await pilot.press("f", "i", "x") + await pilot.press("enter") + await pilot.pause() + # It differs from the repo's stated default, so it goes out as an explicit override. + assert fake.created_agent_clis == [KNOWN_AGENT_CLIS[0]] async def test_pressing_n_with_a_blank_memo_creates_with_none() -> None: @@ -916,8 +958,6 @@ async def test_pressing_n_with_a_blank_memo_creates_with_none() -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("enter") # submit an empty memo await pilot.pause() assert fake.created == [("r1", "spike", None, None)] @@ -939,8 +979,6 @@ async def test_memo_ctrl_s_sets_the_memo_without_submitting() -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("f", "i", "x") # type a memo await pilot.press("ctrl+s") # set without submitting await pilot.pause() @@ -968,8 +1006,6 @@ async def test_memo_ctrl_g_opens_editor_and_updates_textarea(monkeypatch: Any) - await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("h", "i") # type initial text await pilot.press("ctrl+g") # open editor await pilot.pause() @@ -997,8 +1033,6 @@ async def test_memo_textarea_expands_for_multiline_content(monkeypatch: Any) -> await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("ctrl+g") await pilot.pause() await pilot.press("enter") # submit @@ -1024,8 +1058,6 @@ async def test_memo_ctrl_a_attaches_a_file_as_an_artifact(tmp_path: Path) -> Non await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("f", "i", "x") # type a memo await pilot.press("ctrl+a") # open the attach-files modal await pilot.pause() @@ -1066,8 +1098,6 @@ async def test_memo_ctrl_a_preserves_spaces_in_the_filename(tmp_path: Path) -> N await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -1103,8 +1133,6 @@ async def test_memo_ctrl_a_attaches_a_binary_file(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -1138,8 +1166,6 @@ async def test_memo_ctrl_a_can_remove_a_queued_file(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("ctrl+a") # open the attach-files modal await pilot.pause() screen = app.screen @@ -1178,8 +1204,6 @@ async def test_memo_ctrl_a_rejects_a_missing_path(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -1237,8 +1261,6 @@ async def test_memo_ctrl_a_accepts_a_quoted_path(tmp_path: Path) -> None: await pilot.pause() await pilot.press("enter") # workflow await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("ctrl+a") await pilot.pause() screen = app.screen @@ -3575,8 +3597,42 @@ async def test_pressing_j_then_enter_picks_the_second_option_in_a_picker() -> No await pilot.pause() await pilot.press("enter") # workflow: spike (only one) await pilot.pause() - await pilot.press("enter") # agent CLI: the repo default, pre-selected - await pilot.pause() await pilot.press("enter") # submit an empty memo await pilot.pause() assert fake.created == [("r2", "spike", None, None)] + + +async def test_memo_tab_reaches_the_cli_dropdown_and_picking_returns_focus() -> None: + # The interaction the hint promises: tab off the memo onto the dropdown, pick, and land back on + # the memo so Enter still submits (rather than reopening the dropdown). + fake = _FakeClient( + [], + repos=[{"id": "r1", "name": "r1", "git_url": "", "agent_cli": "claude"}], + workflows=[{"name": "spike", "when_to_use": ""}], + ) + app = Dashboard(fake) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("n") + await pilot.pause() + await pilot.press("enter") # repo + await pilot.pause() + await pilot.press("enter") # workflow + await pilot.pause() + memo = app.screen.query_one(MemoTextArea) + select = app.screen.query_one("#memo-cli-select", Select) + assert memo.has_focus # the memo starts focused + await pilot.press("tab") + await pilot.pause() + assert select.has_focus + await pilot.press("enter") # opens the dropdown overlay + await pilot.pause() + await pilot.press("down", "enter") # move off claude → codex + await pilot.pause() + assert select.value == "codex" + assert memo.has_focus # focus handed back, so Enter submits next + await pilot.press("f", "i", "x") + await pilot.press("enter") + await pilot.pause() + assert fake.created == [("r1", "spike", "fix", "fix")] + assert fake.created_agent_clis == ["codex"]