Skip to content

Commit df3d6d8

Browse files
committed
Dashboard: pick a per-task agent CLI in the n new-task flow
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 <noreply@anthropic.com>
1 parent a91661e commit df3d6d8

6 files changed

Lines changed: 230 additions & 38 deletions

File tree

src/panopticon/container/cli/__init__.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88
AgentCLI,
99
get_agent_cli,
1010
register_agent_cli,
11+
registered_agent_clis,
1112
)
1213

13-
__all__ = ["DEFAULT_AGENT_CLI", "AgentCLI", "get_agent_cli", "register_agent_cli"]
14+
__all__ = [
15+
"DEFAULT_AGENT_CLI",
16+
"AgentCLI",
17+
"get_agent_cli",
18+
"register_agent_cli",
19+
"registered_agent_clis",
20+
]

src/panopticon/container/cli/base.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,17 @@ def get_agent_cli(name: str | None = None) -> AgentCLI:
172172
raise KeyError(f"unknown agent CLI {key!r}; registered: {sorted(_REGISTRY)}") from None
173173

174174

175+
def registered_agent_clis() -> tuple[str, ...]:
176+
"""Return the registered adapter names, sorted (loading the built-ins first).
177+
178+
The registry is populated lazily, so callers that only want the *names* need this rather than
179+
reaching into :data:`_REGISTRY` (which is empty until an adapter module is imported). Used by
180+
the drift test that keeps :data:`panopticon.core.models.KNOWN_AGENT_CLIS` honest.
181+
"""
182+
_load_builtin_adapters()
183+
return tuple(sorted(_REGISTRY))
184+
185+
175186
def _load_builtin_adapters() -> None:
176187
"""Register the built-in adapters (imported lazily so this module holds only the contract)."""
177188
from panopticon.container.cli.claude import ClaudeAgentCLI

src/panopticon/core/models.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,15 @@ class Repo:
226226
#: The CLI a task falls back to when neither the task nor its repo names one (ADR 0014 §2/§3).
227227
DEFAULT_AGENT_CLI = "claude"
228228

229+
#: The CLI names an operator can pick from (the dashboard's new-task picker offers these).
230+
#:
231+
#: The set of CLIs is a **control-plane** concept — it drives the base-image variant and the
232+
#: config-dir mount — so it lives here, in the LLM-free core, rather than being read from the
233+
#: adapter registry in :mod:`panopticon.container.cli` (the only LLM-bearing package; importing it
234+
#: from ``terminal``/``taskservice`` would break the determinism invariant). The two are kept in
235+
#: sync by a test — see ``tests/container/test_cli_base.py``.
236+
KNOWN_AGENT_CLIS: tuple[str, ...] = ("claude", "codex")
237+
229238

230239
def resolve_agent_cli(task_agent_cli: str | None, repo_agent_cli: str | None) -> str:
231240
"""Resolve a task's effective agent CLI: its own override → the repo default → ``"claude"``.

src/panopticon/terminal/dashboard.py

Lines changed: 74 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@
105105
from panopticon.client import JsonObj, TaskServiceClient
106106
from panopticon.core.artifacts import InvalidArtifactName, validate_segment
107107
from panopticon.core.dirs import ARTIFACTS_DIR
108-
from panopticon.core.models import resolve_agent_cli
108+
from panopticon.core.models import DEFAULT_AGENT_CLI, KNOWN_AGENT_CLIS, resolve_agent_cli
109109
from panopticon.core.state import TERMINAL_LABELS
110110
from panopticon.sessionservice.local_runner import session_name
111111
from panopticon.taskservice.artifacts_fs import FilesystemArtifactStore
@@ -624,10 +624,13 @@ class _OptionListModal(ModalScreen[_ResultT | None]):
624624
BINDINGS = [("escape", "cancel", "Cancel")]
625625
BOX_ID = "list-box"
626626

627-
def __init__(self, title: str, options: list[str]) -> None:
627+
def __init__(self, title: str, options: list[str], *, initial: str | None = None) -> None:
628628
super().__init__()
629629
self._title = title
630630
self._options = options
631+
#: Which option starts highlighted, so Enter alone picks the expected one. Ignored when it
632+
#: isn't in ``options``.
633+
self._initial = initial
631634

632635
def compose(self) -> ComposeResult:
633636
with Vertical(id=self.BOX_ID):
@@ -639,7 +642,10 @@ def _extra_widgets(self) -> Iterable[Widget]:
639642
return ()
640643

641644
def on_mount(self) -> None:
642-
self.query_one(OptionList).focus()
645+
option_list = self.query_one(OptionList)
646+
if self._initial is not None and self._initial in self._options:
647+
option_list.highlighted = self._options.index(self._initial)
648+
option_list.focus()
643649

644650
def action_cancel(self) -> None:
645651
self.dismiss(None)
@@ -1407,13 +1413,13 @@ def __init__(
14071413

14081414
def _initial(self, name: str) -> str:
14091415
"""A field's pre-populated value: the repo's stored value, else (create mode only)
1410-
``main`` for ``default_base`` / ``claude`` for ``agent_cli``, else blank."""
1416+
``main`` for ``default_base`` / the default CLI for ``agent_cli``, else blank."""
14111417
stored = self._repo.get(name)
14121418
if stored:
14131419
return str(stored)
14141420
if self._editing:
14151421
return ""
1416-
return {"default_base": "main", "agent_cli": "claude"}.get(name, "")
1422+
return {"default_base": "main", "agent_cli": DEFAULT_AGENT_CLI}.get(name, "")
14171423

14181424
def _wf_checked(self, wf: dict[str, Any]) -> bool:
14191425
name = wf["name"]
@@ -1617,7 +1623,7 @@ def create(values: dict[str, Any]) -> str | None:
16171623
capabilities={"docker_in_docker": values["docker_in_docker"]},
16181624
enabled_workflows=values["enabled_workflows"],
16191625
disabled_workflows=values["disabled_workflows"],
1620-
agent_cli=values["agent_cli"] or "claude",
1626+
agent_cli=values["agent_cli"] or DEFAULT_AGENT_CLI,
16211627
)
16221628
except httpx.HTTPStatusError as exc:
16231629
return f"Can't create: {_detail(exc)}"
@@ -1653,7 +1659,7 @@ def save(values: dict[str, Any]) -> str | None:
16531659
capabilities=capabilities,
16541660
enabled_workflows=values["enabled_workflows"],
16551661
disabled_workflows=values["disabled_workflows"],
1656-
agent_cli=values["agent_cli"] or "claude",
1662+
agent_cli=values["agent_cli"] or DEFAULT_AGENT_CLI,
16571663
)
16581664
except httpx.HTTPStatusError as exc:
16591665
return f"Can't update: {_detail(exc)}"
@@ -1985,7 +1991,9 @@ def _load_repo_names(self) -> None:
19851991
try:
19861992
repos = self._client.list_repos()
19871993
self._repo_names = {str(r["id"]): str(r["name"]) for r in repos}
1988-
self._repo_clis = {str(r["id"]): str(r.get("agent_cli") or "claude") for r in repos}
1994+
self._repo_clis = {
1995+
str(r["id"]): str(r.get("agent_cli") or DEFAULT_AGENT_CLI) for r in repos
1996+
}
19891997
except Exception:
19901998
pass
19911999

@@ -2242,11 +2250,20 @@ def on_data_table_row_selected(self, event: DataTable.RowSelected) -> None:
22422250
self.action_refresh()
22432251

22442252
def action_new_task(self) -> None:
2245-
"""`n`: create a task — pick a repo, a workflow, describe the work, then POST it."""
2246-
repos = [str(r["id"]) for r in self._client.list_repos()]
2247-
if not repos:
2253+
"""`n`: create a task — pick a repo, a workflow, an agent CLI, describe the work, then POST it.
2254+
2255+
The CLI step is pre-selected to the repo's own default, so the common path stays
2256+
Enter/Enter/Enter-and-type; picking that default sends no override at all (ADR 0014 §3)."""
2257+
repo_records = self._client.list_repos()
2258+
if not repo_records:
22482259
self.notify("Need at least one repo to create a task.", severity="warning")
22492260
return
2261+
repos = [str(r["id"]) for r in repo_records]
2262+
# Read each repo's CLI from the payload we just fetched rather than `self._repo_clis`, which
2263+
# is only filled in on a refresh pass and so can be empty or stale here.
2264+
repo_clis = {
2265+
str(r["id"]): str(r.get("agent_cli") or DEFAULT_AGENT_CLI) for r in repo_records
2266+
}
22502267

22512268
def pick_workflow(repo: str | None) -> None:
22522269
if repo is None:
@@ -2256,34 +2273,57 @@ def pick_workflow(repo: str | None) -> None:
22562273
self.notify(f"No workflows enabled for repo {repo!r}.", severity="warning")
22572274
return
22582275

2259-
def describe(workflow: str | None) -> None:
2276+
def pick_cli(workflow: str | None) -> None:
22602277
if workflow is None:
22612278
return
2279+
repo_default = repo_clis.get(repo, DEFAULT_AGENT_CLI)
22622280

2263-
def create(result: tuple[str, bool, dict[str, str]] | None) -> None:
2264-
if result is None: # backed out
2281+
def describe(cli: str | None) -> None:
2282+
if cli is None:
22652283
return
2266-
memo_text, submit, artifacts_b64 = result
2267-
stripped = memo_text.strip()
2268-
if _apply_memo_filter(stripped):
2269-
return
2270-
if submit and stripped:
2271-
self._client.create_task(
2272-
repo,
2273-
workflow,
2274-
stripped,
2275-
initial_prompt=stripped,
2276-
artifacts_b64=artifacts_b64,
2277-
)
2278-
else:
2279-
self._client.create_task(
2280-
repo, workflow, stripped or None, artifacts_b64=artifacts_b64
2281-
)
2282-
self.action_refresh()
2283-
2284-
self.push_screen(MemoScreen(), create)
2284+
# None = "no override, use the repo default" — what `resolve_agent_cli` expects.
2285+
agent_cli = None if cli == repo_default else cli
2286+
2287+
def create(result: tuple[str, bool, dict[str, str]] | None) -> None:
2288+
if result is None: # backed out
2289+
return
2290+
memo_text, submit, artifacts_b64 = result
2291+
stripped = memo_text.strip()
2292+
if _apply_memo_filter(stripped):
2293+
return
2294+
if submit and stripped:
2295+
self._client.create_task(
2296+
repo,
2297+
workflow,
2298+
stripped,
2299+
initial_prompt=stripped,
2300+
artifacts_b64=artifacts_b64,
2301+
agent_cli=agent_cli,
2302+
)
2303+
else:
2304+
self._client.create_task(
2305+
repo,
2306+
workflow,
2307+
stripped or None,
2308+
artifacts_b64=artifacts_b64,
2309+
agent_cli=agent_cli,
2310+
)
2311+
self.action_refresh()
2312+
2313+
self.push_screen(MemoScreen(), create)
2314+
2315+
# The default is named in the title, not baked into an option label — ChoiceScreen
2316+
# dismisses the option's text, so a decorated label would need stripping back off.
2317+
self.push_screen(
2318+
ChoiceScreen(
2319+
f"agent CLI (repo default: {repo_default})",
2320+
list(KNOWN_AGENT_CLIS),
2321+
initial=repo_default,
2322+
),
2323+
describe,
2324+
)
22852325

2286-
self.push_screen(WorkflowScreen(workflows), describe)
2326+
self.push_screen(WorkflowScreen(workflows), pick_cli)
22872327

22882328
self.push_screen(ChoiceScreen("repo", repos), pick_workflow)
22892329

tests/container/test_cli_base.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313
from panopticon.container.cli import (
1414
DEFAULT_AGENT_CLI,
1515
AgentCLI,
16+
base,
1617
get_agent_cli,
1718
register_agent_cli,
19+
registered_agent_clis,
1820
)
1921
from panopticon.container.cli.claude import ClaudeAgentCLI
2022
from panopticon.container.cli.codex import CodexAgentCLI
23+
from panopticon.core import models as core_models
2124

2225

2326
def test_default_resolves_to_claude() -> None:
@@ -26,6 +29,17 @@ def test_default_resolves_to_claude() -> None:
2629
assert isinstance(get_agent_cli("claude"), ClaudeAgentCLI)
2730

2831

32+
def test_core_agent_cli_constants_match_the_adapter_registry() -> None:
33+
"""The control plane can't import this package at runtime (the determinism invariant — `core`
34+
stays LLM-free), so `core.models` re-states the CLI name list and the default. This is the guard
35+
that keeps the two copies from drifting: add an adapter, and it fails until `core` knows about it.
36+
"""
37+
assert list(registered_agent_clis()) == sorted(core_models.KNOWN_AGENT_CLIS)
38+
assert core_models.DEFAULT_AGENT_CLI == DEFAULT_AGENT_CLI
39+
# The fallback must itself be a selectable CLI.
40+
assert core_models.DEFAULT_AGENT_CLI in core_models.KNOWN_AGENT_CLIS
41+
42+
2943
def test_codex_is_a_registered_built_in_adapter() -> None:
3044
# The second built-in CLI: registering it makes it resolvable with no launcher edit (ADR 0014 §2).
3145
assert isinstance(get_agent_cli("codex"), CodexAgentCLI)
@@ -73,8 +87,13 @@ def launch(self, config_dir: Path) -> None: # pragma: no cover - not exercised
7387
pass
7488

7589
register_agent_cli(_Fake)
76-
resolved = get_agent_cli("fake-cli")
77-
assert isinstance(resolved, _Fake) and resolved.config_dirname == ".fake"
90+
try:
91+
resolved = get_agent_cli("fake-cli")
92+
assert isinstance(resolved, _Fake) and resolved.config_dirname == ".fake"
93+
finally:
94+
# The registry is module-global: leaving the fake in it would leak into any other test that
95+
# inspects the registered names (e.g. the KNOWN_AGENT_CLIS drift guard above).
96+
base._REGISTRY.pop("fake-cli", None)
7897

7998

8099
# -- shared base-class behaviour ----------------------------------------------------------------

0 commit comments

Comments
 (0)