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..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 @@ -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 @@ -717,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``).""" @@ -735,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; } """ @@ -746,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") @@ -766,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) @@ -781,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) @@ -1407,13 +1451,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 +1661,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 +1697,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 +2029,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 +2288,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, describe the work, then POST it. + + 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") 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: @@ -2259,14 +2314,17 @@ def pick_workflow(repo: str | None) -> None: def describe(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: + def create(result: tuple[str, bool, dict[str, str], str] | None) -> None: if result is None: # backed out return - memo_text, submit, artifacts_b64 = result + 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, @@ -2274,14 +2332,21 @@ def create(result: tuple[str, bool, dict[str, str]] | None) -> None: 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 + repo, + workflow, + stripped or None, + artifacts_b64=artifacts_b64, + agent_cli=agent_cli, ) self.action_refresh() - self.push_screen(MemoScreen(), create) + self.push_screen( + MemoScreen(agent_clis=KNOWN_AGENT_CLIS, repo_default=repo_default), create + ) self.push_screen(WorkflowScreen(workflows), describe) 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..b7e1d3af 100644 --- a/tests/terminal/test_dashboard.py +++ b/tests/terminal/test_dashboard.py @@ -16,13 +16,16 @@ import pytest from textual.app import App 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 from panopticon.terminal.dashboard import ( _ENSEMBLE_KEY_PREFIX, _INDEFINITE_SNOOZE_UNTIL, _SNOOZE_DURATION, Dashboard, + MemoTextArea, SpaceCheckbox, TaskDetailScreen, _dim, @@ -115,6 +118,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 +240,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]: @@ -815,6 +821,126 @@ async def test_pressing_n_creates_a_task_via_repo_workflow_then_memo() -> None: 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_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"}], + 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 == "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_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"}], + 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() + app.screen.query_one("#memo-cli-select", Select).value = "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_memo_cli_dropdown_offers_every_known_cli() -> 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) + # 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: @@ -3474,3 +3600,39 @@ async def test_pressing_j_then_enter_picks_the_second_option_in_a_picker() -> No 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"]