diff --git a/clients/terminal/api.py b/clients/terminal/api.py index b7a9b92..b735939 100644 --- a/clients/terminal/api.py +++ b/clients/terminal/api.py @@ -66,6 +66,19 @@ return resp.json() +def get_todos(session_id: str) -> dict: + """Fetch the session's current todo list (GET /sessions/{id}/todos). + + Returns ``{"session_id": ..., "tasks": [{"index", "text", "status", "validation"}, ...]}``. + Used to seed the side-panel todo on attach/switch before the first + ``todo_updated`` WS event arrives. + """ + with _client() as client: + resp = client.get(f"/sessions/{session_id}/todos") + resp.raise_for_status() + return resp.json() + + def stop_session(session_id: str) -> dict: with _client() as client: resp = client.post(f"/sessions/{session_id}/stop") diff --git a/clients/terminal/tui/tui_app.py b/clients/terminal/tui/tui_app.py index a5f53ce..32acb12 100644 --- a/clients/terminal/tui/tui_app.py +++ b/clients/terminal/tui/tui_app.py @@ -27,7 +27,7 @@ from clients.terminal.tui.commands.registry import get_registry from clients.terminal.tui.screens.command_palette import CommandPaletteScreen from clients.terminal.tui.screens.permission_dialog import PermissionDialogScreen -from clients.terminal.tui.widgets import ChatPanel, InputBox, StatusBar, StatusPanel +from clients.terminal.tui.widgets import ChatPanel, InputBox, StatusBar, StatusPanel, TodoPanel from clients.terminal.tui.ws_bridge import WsBridge @@ -59,6 +59,7 @@ set_active_theme(self._theme_name) self._chat_panel = ChatPanel() self._status_panel = StatusPanel() + self._todo_panel = TodoPanel() self._input_box = InputBox() self._status_bar = StatusBar() self._state = StateManager() @@ -83,6 +84,7 @@ yield self._chat_panel with Vertical(): yield self._status_panel + yield self._todo_panel yield self._input_box yield self._status_bar @@ -107,6 +109,8 @@ self.theme = self._theme_name if self._status_panel: self._status_panel.set_theme(self._theme_name) + if self._todo_panel: + self._todo_panel.refresh_content() async def _startup(self) -> None: session_id = await self._resolve_session( @@ -186,6 +190,13 @@ ) self._status_panel.set_backend(settings.base_url) self._status_panel.set_theme(self._theme_name) + # Seed the side-panel todo from the server so a resumed/switched session + # shows its existing plan before the first todo_updated WS event arrives. + try: + todos = api.get_todos(session_id) + self._todo_panel.set_tasks(todos.get("tasks") or []) + except Exception: + self._todo_panel.clear() # Replay the session's past conversation before the connection banner # so a resumed/switched session shows its history instead of a blank chat. self._chat_panel.load_history(history) @@ -391,6 +402,11 @@ if model: self._status_panel.set_model(model) return + elif msg_type == "todo_updated": + # Live todo plan update from the agent (after planning auto-populate + # or a todo tool call). Renders in the side panel; not a chat item. + self._todo_panel.set_tasks(payload.get("tasks") or []) + return elif msg_type in ("compression_started", "context_compressed"): # Compression carries a fresh context-token count (post-compress on # context_compressed, pre-compress on compression_started). Update diff --git a/clients/terminal/tui/widgets/__init__.py b/clients/terminal/tui/widgets/__init__.py index 2ec28d3..46353c8 100644 --- a/clients/terminal/tui/widgets/__init__.py +++ b/clients/terminal/tui/widgets/__init__.py @@ -7,5 +7,6 @@ from .input_box import InputBox from .status_bar import StatusBar from .status_panel import StatusPanel +from .todo_list import TodoList, TodoPanel -__all__ = ["ChatPanel", "CommandHints", "InputBox", "StatusBar", "StatusPanel"] +__all__ = ["ChatPanel", "CommandHints", "InputBox", "StatusBar", "StatusPanel", "TodoList", "TodoPanel"] diff --git a/clients/terminal/tui/widgets/status_panel.py b/clients/terminal/tui/widgets/status_panel.py index 6183d8c..969bf89 100644 --- a/clients/terminal/tui/widgets/status_panel.py +++ b/clients/terminal/tui/widgets/status_panel.py @@ -11,7 +11,11 @@ class StatusPanel(Vertical): - """Right-side panel showing session/profile/model/connection info.""" + """Right-side info block: session/profile/model/connection. + + Auto-height — shrinks to its content so the TodoPanel below it fills the + rest of the right column. + """ DEFAULT_CSS = """ StatusPanel { @@ -19,7 +23,7 @@ background: $tui-panel; color: $tui-text-muted; padding: 1; - height: 1fr; + height: auto; width: 1fr; } StatusPanel .title { @@ -52,7 +56,6 @@ yield self._connection yield self._backend yield self._theme - yield Static("", classes="spacer") yield self._hint def set_profile(self, profile_id: str) -> None: diff --git a/clients/terminal/tui/widgets/todo_list.py b/clients/terminal/tui/widgets/todo_list.py new file mode 100644 index 0000000..a6cd153 --- /dev/null +++ b/clients/terminal/tui/widgets/todo_list.py @@ -0,0 +1,151 @@ +"""Todo list side-panel widget for the TUI. + +Renders the agent's current todo plan (the parent session's todo row) with +status-coloured markers and a progress header. Fed by: + + * ``todo_updated`` WS events (live, after each tool turn / planning auto-populate) + * ``api.get_todos`` (initial seed on attach/switch, before the first WS event) + +Sub-agent todos are NOT shown here yet — they live in isolated KV rows and will +be surfaced as nested sub-lists in Phase 3. + +Layout: ``TodoPanel`` (a scrollable container, ``height: 1fr``) wraps a +``TodoList`` (a plain ``Static``, ``height: auto``). The info block above +shrinks to its content; the todo block fills the rest of the right column and +scrolls when the plan is longer than the viewport. +""" + +from __future__ import annotations + +from textual.app import ComposeResult +from textual.containers import ScrollableContainer +from textual.widgets import Static + +from clients.terminal.tui.themes import get_active_theme + +_STATUS_ICON: dict[str, str] = { + "pending": "○", + "in_progress": "◎", + "done": "✓", + "failed": "✗", + "skipped": "—", +} + +# Theme attribute name (on the active theme) used to colour each status marker. +_STATUS_COLOR_ATTR: dict[str, str] = { + "pending": "text_dim", + "in_progress": "accent", + "done": "success", + "failed": "error", + "skipped": "text_dim", +} + + +class TodoList(Static): + """Renders the live todo plan as coloured, status-marked lines. + + A plain Static whose content is rebuilt from the latest task payload via + ``set_tasks`` / ``clear`` / ``refresh_content`` (no compose override, and + never a method named ``_render`` — that shadows Widget._render and crashes + Textual's layout pass). + """ + + DEFAULT_CSS = """ + TodoList { + height: auto; + color: $tui-text-muted; + padding: 0; + } + """ + + def __init__(self) -> None: + super().__init__("[dim]No plan yet[/dim]") + self._tasks: list[dict] = [] + + def set_tasks(self, tasks: list[dict]) -> None: + """Replace the rendered todo. ``tasks`` is the REST/WS payload: + ``[{"index", "text", "status", "validation"}, ...]``.""" + self._tasks = list(tasks) + self.update(self._build_content()) + + def clear(self) -> None: + self._tasks = [] + self.update(self._build_content()) + + def refresh_content(self) -> None: + """Re-render with the current tasks (used after a theme switch so the + status-marker colours pick up the new palette).""" + self.update(self._build_content()) + + def _build_content(self) -> str: + theme = get_active_theme() + dim = theme.text_dim.hex + if not self._tasks: + return f"[{dim}]No plan yet[/]" + n = len(self._tasks) + done = sum(1 for t in self._tasks if t.get("status") == "done") + # Progress header: count in the done colour, total dim. + lines: list[str] = [ + f"[{theme.success.hex}]{done}[/{theme.success.hex}]/{n} done" + ] + for t in self._tasks: + status = t.get("status", "") or "pending" + icon = _STATUS_ICON.get(status, "?") + icon_color = getattr(theme, _STATUS_COLOR_ATTR.get(status, "text_dim"), theme.text_dim).hex + text = t.get("text", "") + # Line emphasis: the active step is bold + bright; completed/skipped + # are dimmed; failed pops in the warning colour; pending is muted. + if status == "in_progress": + text_style = f"bold {theme.text.hex}" + elif status in ("done", "skipped"): + text_style = f"dim {theme.text_muted.hex}" + elif status == "failed": + text_style = theme.warning.hex + else: + text_style = theme.text_muted.hex + suffix = f" [{dim}]({status})[/]" if status not in ("pending", "done") else "" + note = f" [{dim}]✓ {t['validation']}[/]" if t.get("validation") else "" + lines.append( + f" [{icon_color}]{icon}[/] [{text_style}]{t.get('index', '')}. {text}[/]{suffix}{note}" + ) + return "\n".join(lines) + + +class TodoPanel(ScrollableContainer): + """Right-column scrollable frame around the todo plan. + + ``height: 1fr`` so it fills whatever the info block above leaves free; the + inner ``TodoList`` is ``height: auto`` and this container scrolls when the + plan is longer than the viewport. + """ + + DEFAULT_CSS = """ + TodoPanel { + border: solid $tui-border; + background: $tui-panel; + color: $tui-text-muted; + padding: 0 1; + height: 1fr; + width: 1fr; + } + """ + + def __init__(self) -> None: + super().__init__() + self._todo = TodoList() + + def compose(self) -> ComposeResult: + yield self._todo + + def on_mount(self) -> None: + self._todo.styles.height = "auto" + self.border_title = "TODO" + + def set_tasks(self, tasks: list[dict]) -> None: + self._todo.set_tasks(tasks) + + def clear(self) -> None: + self._todo.clear() + + def refresh_content(self) -> None: + self._todo.refresh_content() \ No newline at end of file diff --git a/navi/api/routes/sessions.py b/navi/api/routes/sessions.py index 5e9392e..2c65e32 100644 --- a/navi/api/routes/sessions.py +++ b/navi/api/routes/sessions.py @@ -13,12 +13,12 @@ from navi.api.deps import ( get_backend_registry, + get_kv_store, get_memory_store, get_profile_registry, get_scheduler, get_session_store, require_admin, - require_permission, require_user, ) from navi.auth import User @@ -31,6 +31,7 @@ from navi.exceptions import ProfileNotFound from navi.memory import MemoryStore from navi.session_files import delete_session_dir, ensure_session_dir, is_forbidden, list_session_files, safe_filename, session_dir +from navi.store import KvStore log = structlog.get_logger() @@ -222,6 +223,29 @@ } +@router.get("/{session_id}/todos") +async def get_session_todos( + session_id: str, + store: Annotated[SessionStore, Depends(get_session_store)], + kv: Annotated[KvStore, Depends(get_kv_store)], + user: Annotated[User, Depends(require_user)], +) -> dict: + """Return the session's current todo list (the parent agent's plan). + + Reads the parent session's KV row via explicit (user_id, session_id) — no + ContextVars. Sub-agent todos live in isolated rows and are not exposed here + (Phase 3). Used by the TUI to seed the side panel on attach/switch. + """ + from navi.tools.todo import load_tasks_for + + session = await store.get(session_id) + if session is None: + raise HTTPException(status_code=404, detail="Session not found") + check_session_access(session, user, permission="navi.sessions.read_all") + tasks = await load_tasks_for(session_id, session.user_id, kv=kv) + return {"session_id": session_id, "tasks": tasks} + + @router.patch("/{session_id}/pin") async def pin_session( session_id: str, diff --git a/navi/core/agent.py b/navi/core/agent.py index ec2d498..fd0e044 100644 --- a/navi/core/agent.py +++ b/navi/core/agent.py @@ -65,6 +65,7 @@ TextDelta, ThinkingDelta, ThinkingEnd, + TodoUpdated, ToolEvent, ToolStarted, ) @@ -497,6 +498,11 @@ else: yield _ev + # Planning auto-populates the todo (planning.set_tasks) when Phase 3 + # produced steps. Push the fresh state to the UI so the side panel + # reflects the plan before the tool-calling loop starts. + yield await self._todo_updated(session) + ctx_task = asyncio.create_task(self._ctx_builder._collect_context_injections(profile)) mem_facts_task = asyncio.create_task( self._ctx_builder._memory_facts_msg( @@ -680,6 +686,10 @@ ): yield _ev + # The todo tool may have been called this turn (set/update/clear). + # Re-read and push the latest state to the UI's side panel. + yield await self._todo_updated(session) + # 6. Cooperative stop: check after tool execution before next LLM call if stop_event and stop_event.is_set(): await self._sessions.save(session) @@ -757,6 +767,18 @@ def _get_backend(self, backend_key: str) -> LLMBackend: return self._backends.get(backend_key) + async def _todo_updated(self, session) -> TodoUpdated: + """Build a TodoUpdated event from the session's current todo state. + + Reads via explicit (user_id, session_id) so it works regardless of the + ContextVar state at the call site. Sub-agent todos are isolated in their + own KV row and never reach here (the parent's session.id is the parent's). + """ + from navi.tools.todo import load_tasks_for + + tasks = await load_tasks_for(session.id, session.user_id) + return TodoUpdated(session_id=session.id, tasks=tasks) + async def _compression_events_preturn(self, session, llm, profile, session_id): if ( settings.context_compression_enabled diff --git a/navi/core/events.py b/navi/core/events.py index 2f90374..2fc70ea 100644 --- a/navi/core/events.py +++ b/navi/core/events.py @@ -243,6 +243,28 @@ @dataclass +class TodoUpdated: + """Emitted when the session's todo list changes — auto-populated from the + plan at the start of a task, or via the todo tool during execution. + + Forwarded to WebSocket clients so the UI can render the live todo in the + side panel. Sub-agent todos are NOT emitted here: they live in an isolated + KV row scoped to the sub-agent's run id (Phase 3 will surface those as + nested sub-lists). Additive — older clients ignore the unknown type. + """ + + session_id: str + tasks: list = field(default_factory=list) # [{index, text, status, validation}, ...] + + def to_wire(self) -> dict: + return { + "type": "todo_updated", + "session_id": self.session_id, + "tasks": self.tasks, + } + + +@dataclass class SubagentComplete: """Internal: emitted by run_ephemeral into the parent sink to report metrics. Never forwarded to WebSocket clients.""" @@ -363,5 +385,5 @@ ToolStarted | ToolEvent | TextDelta | ThinkingDelta | ThinkingEnd | StreamEnd | StreamStopped | CompressionStarted | ContextCompressed | TurnThinking | ProfileSwitched | PlanningStatus | PlanReady | SubagentComplete | AIHelperTokensUsed | PlanningDebugData - | RecallUpdate | McpStatusUpdate | TerminalOutputDelta | TerminalClosed + | RecallUpdate | McpStatusUpdate | TerminalOutputDelta | TerminalClosed | TodoUpdated ) diff --git a/navi/core/planning.py b/navi/core/planning.py index b37aa6b..79f8dc9 100644 --- a/navi/core/planning.py +++ b/navi/core/planning.py @@ -455,8 +455,13 @@ if _todo_steps: try: from navi.tools.todo import set_tasks - from navi.tools._internal.base import current_session_id as _sid_var - _sid = _sid_var.get() or "__default__" + from navi.tools._internal.base import ( + current_session_id as _sid_var, + current_todo_session_id as _todo_sid_var, + ) + # Sub-agents set current_todo_session_id to their run id; use it so the + # sub-agent's plan populates its own todo row, not the parent session's. + _sid = _todo_sid_var.get() or _sid_var.get() or "__default__" await set_tasks(_sid, _todo_steps) log.debug("agent.todo_auto_populated", steps=len(_todo_steps), session=_sid) except Exception: diff --git a/navi/core/subagent_runner.py b/navi/core/subagent_runner.py index 3ec7c23..0a890e0 100644 --- a/navi/core/subagent_runner.py +++ b/navi/core/subagent_runner.py @@ -10,7 +10,6 @@ import structlog from navi.config import settings -from navi.exceptions import ContextTooLargeError from navi.llm.base import LLMBackend, Message, ToolCallRequest from navi.tools._internal.base import ToolContext, current_event_sink, current_stop_event @@ -86,6 +85,7 @@ from navi.tools._internal.base import ( current_session_id as _sid_var, current_model as _model_var, + current_todo_session_id as _todo_sid_var, current_user_id as _uid_var, current_user_role as _role_var, current_user_info as _uinfo_var, @@ -93,12 +93,17 @@ _prev_sid = _sid_var.get(None) _prev_model = _model_var.get(None) + _prev_todo_sid = _todo_sid_var.get(None) _prev_uid = _uid_var.get(None) _prev_role = _role_var.get() _prev_uinfo = _uinfo_var.get(None) subagent_run_id = f"subagent_{uuid.uuid4().hex[:12]}" tool_session_id = parent_session_id or subagent_run_id _sid_var.set(tool_session_id) + # Scope the sub-agent's todo list to its own ephemeral run id so its + # auto-populated plan and todo updates do NOT clobber the parent session's + # todo (which the parent's goal-anchoring reads every iteration). + _todo_sid_var.set(subagent_run_id) profile = self._profiles.get(profile_id) _model_var.set(profile.model) @@ -444,6 +449,7 @@ finally: _sid_var.set(_prev_sid) _model_var.set(_prev_model) + _todo_sid_var.set(_prev_todo_sid) _uid_var.set(_prev_uid) _role_var.set(_prev_role) _uinfo_var.set(_prev_uinfo) diff --git a/navi/tools/_internal/base.py b/navi/tools/_internal/base.py index a8834c8..fda757b 100644 --- a/navi/tools/_internal/base.py +++ b/navi/tools/_internal/base.py @@ -18,6 +18,14 @@ # Set by Agent before every tool call. Tools that need per-session state read this. current_session_id: ContextVar[str | None] = ContextVar("current_session_id", default=None) +# Optional override for the session id used to scope the todo list. Sub-agents set +# this to their ephemeral run id so their auto-populated plan and todo updates land in +# an isolated KV row instead of clobbering the parent session's todo. When unset +# (parent agent run), todo falls back to current_session_id. +current_todo_session_id: ContextVar[str | None] = ContextVar( + "current_todo_session_id", default=None +) + # Set by run_stream() before executing a tool. run_ephemeral() reads this to forward # sub-agent tool events up to the parent WS stream. current_event_sink: ContextVar[asyncio.Queue | None] = ContextVar( diff --git a/navi/tools/todo.py b/navi/tools/todo.py index 14a7ab2..8ca624f 100644 --- a/navi/tools/todo.py +++ b/navi/tools/todo.py @@ -2,9 +2,17 @@ from __future__ import annotations import json -from dataclasses import dataclass, field +from dataclasses import dataclass -from navi.tools._internal.base import Tool, ToolContext, ToolResult, current_session_id, current_user_id +from navi.llm.base import Message +from navi.tools._internal.base import ( + Tool, + ToolContext, + ToolResult, + current_session_id, + current_todo_session_id, + current_user_id, +) _STATUS_ICON: dict[str, str] = { "pending": "○", @@ -32,7 +40,17 @@ def _sid(explicit: str | None = None) -> str: - return explicit or current_session_id.get() or "__default__" + # Sub-agents set current_todo_session_id to their ephemeral run id so their + # plan/todo stay isolated from the parent session's todo. It must win over + # ctx.session_id (which is still the parent session id for a sub-agent, since + # other tools rely on it). The parent agent run leaves it unset, so todo falls + # back to the explicit / regular session id. + return ( + current_todo_session_id.get() + or explicit + or current_session_id.get() + or "__default__" + ) def _uid(explicit: str | None = None) -> str | None: @@ -166,7 +184,7 @@ success=True, output=( self._render(sid, tasks) + "\n\n" - f"[Tip: next time add a 'validation' field when marking a step failed — " + "[Tip: next time add a 'validation' field when marking a step failed — " "describe what went wrong and what you tried. " "This helps with re-planning.]" ), @@ -222,7 +240,7 @@ return frozenset() -async def get_progress_message(session_id: str, *, first_iteration: bool = False) -> "Message | None": +async def get_progress_message(session_id: str, *, first_iteration: bool = False) -> Message | None: """Build a compact system reminder with current todo state.""" try: tasks = await _load_tasks(session_id) @@ -279,7 +297,6 @@ "Do not start the next task until the current one is verified and marked done." ) - from navi.llm.base import Message return Message(role="system", content="\n".join(lines)) except Exception: return None @@ -291,6 +308,38 @@ await _save_tasks(session_id, tasks) +async def load_tasks_for( + session_id: str, user_id: str | None, kv=None +) -> list[dict]: + """Load the todo list for an explicit (user_id, session_id) pair as + JSON-serialisable dicts — no ContextVars. + + Used by the REST endpoint (GET /sessions/{id}/todos) and the TodoUpdated + emitter, both of which run outside tool execution where the per-session + ContextVars are not reliably set. Pass ``kv`` to bypass the module-global + store (the REST route resolves its own KvStore via deps). + """ + store = kv if kv is not None else _kv_store + if store is None: + return [] + raw = await store.get(user_id, session_id, "todo", "tasks") + if not raw: + return [] + try: + data = json.loads(raw) + except Exception: + return [] + return [ + { + "index": i + 1, + "text": item.get("text", ""), + "status": item.get("status", "pending"), + "validation": item.get("validation", ""), + } + for i, item in enumerate(data) + ] + + async def render_todo_lines(session_id: str) -> list[str]: """Return a list of formatted todo lines for goal anchoring.""" tasks = await _load_tasks(session_id) diff --git a/tests/clients/test_todo_list.py b/tests/clients/test_todo_list.py new file mode 100644 index 0000000..88947bc --- /dev/null +++ b/tests/clients/test_todo_list.py @@ -0,0 +1,141 @@ +"""Tests for the TodoList side-panel widget.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from rich.text import Text + +from clients.terminal.tui.widgets.todo_list import TodoList + + +def _plain(content: str) -> str: + """Strip Rich markup so assertions see the visible text, not colour tags.""" + return Text.from_markup(content).plain + + +@pytest.fixture(autouse=True) +def tmp_state_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Override the state dir so tests never touch ~/.navi_code.""" + from clients.terminal import config + + original = config.settings.state_dir + config.settings.state_dir = tmp_path + import clients.terminal.tui.settings as settings_module + + settings_module._tui_settings = None + yield tmp_path + config.settings.state_dir = original + settings_module._tui_settings = None + + +@pytest.mark.anyio +async def test_todo_list_empty_state() -> None: + """A fresh TodoList shows the 'No plan yet' placeholder.""" + from clients.terminal.tui.tui_app import NaviCodeTui + + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + todo = pilot.app.query_one(TodoList) + # _build_content is the source of truth (never name this _render — + # it shadows Widget._render and crashes Textual's layout pass). + assert "No plan yet" in todo._build_content() + + +@pytest.mark.anyio +async def test_todo_list_renders_tasks_with_header() -> None: + from clients.terminal.tui.tui_app import NaviCodeTui + + tasks = [ + {"index": 1, "text": "step one", "status": "done", "validation": "ran tests"}, + {"index": 2, "text": "step two", "status": "in_progress", "validation": ""}, + {"index": 3, "text": "step three", "status": "failed", "validation": "boom"}, + ] + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + todo = pilot.app.query_one(TodoList) + todo.set_tasks(tasks) + plain = _plain(todo._build_content()) + assert "1/3 done" in plain + assert "step one" in plain + assert "step two" in plain + assert "(in_progress)" in plain + assert "(failed)" in plain + assert "✓ ran tests" in plain # validation note on the done step + assert "✓ boom" in plain # validation note on the failed step + + +@pytest.mark.anyio +async def test_todo_list_clear_resets_to_placeholder() -> None: + from clients.terminal.tui.tui_app import NaviCodeTui + + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + todo = pilot.app.query_one(TodoList) + todo.set_tasks([{"index": 1, "text": "x", "status": "pending", "validation": ""}]) + assert "x" in todo._build_content() + todo.clear() + assert "No plan yet" in todo._build_content() + + +def test_build_content_never_named_render(): + """Guard against the Textual _render shadow pitfall: a method called _render + on a Widget subclass crashes the layout pass (AttributeError get_height).""" + assert "_render" not in TodoList.__dict__, ( + "TodoList must not override _render — it shadows Widget._render " + "(Textual calls it expecting a Strip, gets a str → AttributeError)." + ) + assert "_build_content" in TodoList.__dict__ + + +@pytest.mark.anyio +async def test_status_markers_are_coloured_and_emphasised() -> None: + """Each status gets its own marker colour, and the active step is bolded + while completed/skipped steps are dimmed — so progress is legible at a glance.""" + from clients.terminal.tui.tui_app import NaviCodeTui + from clients.terminal.tui.themes import get_active_theme + + tasks = [ + {"index": 1, "text": "done step", "status": "done", "validation": ""}, + {"index": 2, "text": "active step", "status": "in_progress", "validation": ""}, + {"index": 3, "text": "failed step", "status": "failed", "validation": ""}, + {"index": 4, "text": "queued step", "status": "pending", "validation": ""}, + ] + theme = get_active_theme() + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + todo = pilot.app.query_one(TodoList) + todo.set_tasks(tasks) + content = todo._build_content() + # Marker icons are wrapped in their status colour. + assert f"[{theme.success.hex}]✓" in content # done + assert f"[{theme.accent.hex}]◎" in content # in_progress + assert f"[{theme.error.hex}]✗" in content # failed + assert f"[{theme.text_dim.hex}]○" in content # pending + # The active step is bold; completed steps are dimmed. + assert "bold" in content + assert "dim" in content + # Plain text still carries every step. + plain = _plain(content) + assert "active step" in plain + assert "queued step" in plain + + +@pytest.mark.anyio +async def test_todo_panel_is_scrollable_and_wraps_list() -> None: + """The todo lives in its own scrollable panel below the info block, not + inside StatusPanel.""" + from textual.containers import ScrollableContainer + + from clients.terminal.tui.tui_app import NaviCodeTui + from clients.terminal.tui.widgets.todo_list import TodoPanel + + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + panel = pilot.app.query_one(TodoPanel) + assert isinstance(panel, ScrollableContainer) + # The TodoList is a child of the panel. + assert panel.query_one(TodoList) is not None + # The panel fills the remaining column height; the info block shrinks. + assert str(panel.styles.height) == "1fr" \ No newline at end of file diff --git a/tests/clients/test_tui_app.py b/tests/clients/test_tui_app.py index 2902774..fb8de38 100644 --- a/tests/clients/test_tui_app.py +++ b/tests/clients/test_tui_app.py @@ -363,6 +363,71 @@ @pytest.mark.anyio +async def test_todo_updated_event_renders_in_status_panel() -> None: + """A todo_updated WS event updates the TodoList in the side panel.""" + from clients.terminal.tui.widgets.todo_list import TodoList + + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + app = pilot.app + tasks = [ + {"index": 1, "text": "first step", "status": "done", "validation": "ok"}, + {"index": 2, "text": "second step", "status": "in_progress", "validation": ""}, + ] + app.on_ws_event(WsEvent({"type": "todo_updated", "session_id": "x", "tasks": tasks})) + await pilot.pause() + todo = app.query_one(TodoList) + from rich.text import Text as _Text + + plain = _Text.from_markup(todo._build_content()).plain + assert "1/2 done" in plain + assert "first step" in plain + assert "second step" in plain + + +@pytest.mark.anyio +async def test_todo_updated_not_forwarded_to_chat() -> None: + """todo_updated is a side-panel update, not a chat item.""" + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + app = pilot.app + app.on_ws_event( + WsEvent({"type": "todo_updated", "session_id": "x", "tasks": [{"index": 1, "text": "t", "status": "pending", "validation": ""}]}) + ) + await pilot.pause() + chat = app.query_one("ChatPanel") + assert not any( + getattr(item, "kind", None) == "todo_updated" for item in chat._model.items + ) + + +@pytest.mark.anyio +async def test_attach_session_seeds_todo_from_api(monkeypatch) -> None: + """attach_session fetches the existing todo via api.get_todos so a resumed + session shows its plan before the first todo_updated WS event.""" + from clients.terminal.tui.widgets.todo_list import TodoList + + fetched: list[str] = [] + + def fake_get_todos(session_id: str) -> dict: + fetched.append(session_id) + return { + "session_id": session_id, + "tasks": [{"index": 1, "text": "seeded step", "status": "pending", "validation": ""}], + } + + import clients.terminal.api as api_module + + monkeypatch.setattr(api_module, "get_todos", fake_get_todos) + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + # The startup worker attached a session, which should have fetched todos. + assert fetched, "attach_session did not call api.get_todos" + todo = pilot.app.query_one(TodoList) + assert "seeded step" in todo._build_content() + + +@pytest.mark.anyio async def test_streaming_state_tracks_ws_events() -> None: """stream_start enters streaming mode; stream_end/stream_stopped/error leave it.""" async with NaviCodeTui(new_session=True).run_test() as pilot: diff --git a/tests/integration/test_api_routes.py b/tests/integration/test_api_routes.py index b0a52f9..e7f57e4 100644 --- a/tests/integration/test_api_routes.py +++ b/tests/integration/test_api_routes.py @@ -105,6 +105,61 @@ assert response.status_code == 404 @pytest.mark.anyio + async def test_get_todos_empty(self, client, make_session): + session = await make_session("secretary") + response = client.get(f"/sessions/{session.id}/todos") + assert response.status_code == 200 + data = response.json() + assert data["session_id"] == session.id + assert data["tasks"] == [] + + @pytest.mark.anyio + async def test_get_todos_returns_plan(self, client, make_session, monkeypatch): + """GET /sessions/{id}/todos reads the parent session's todo KV row.""" + import json as _json + + session = await make_session("secretary") + + class _MemKv: + def __init__(self): + self._d = {} + + async def get(self, user_id, session_id, scope, key): + return self._d.get((user_id or "", session_id, scope, key)) + + async def set(self, user_id, session_id, scope, key, value): + self._d[(user_id or "", session_id, scope, key)] = value + + fake_kv = _MemKv() + await fake_kv.set( + session.user_id, session.id, "todo", "tasks", + _json.dumps([ + {"text": "step one", "status": "done", "validation": "ran tests"}, + {"text": "step two", "status": "in_progress", "validation": ""}, + ]), + ) + # The route resolves its KvStore via get_kv_store() → container.kv_store. + # Inject our fake so the route reads from it (kv arg, not the global). + from navi.api.deps import _resolve_container + + container = _resolve_container() + container.kv_store = fake_kv + try: + response = client.get(f"/sessions/{session.id}/todos") + finally: + container.kv_store = None + assert response.status_code == 200 + tasks = response.json()["tasks"] + assert tasks == [ + {"index": 1, "text": "step one", "status": "done", "validation": "ran tests"}, + {"index": 2, "text": "step two", "status": "in_progress", "validation": ""}, + ] + + def test_get_todos_not_found(self, client): + response = client.get("/sessions/nonexistent/todos") + assert response.status_code == 404 + + @pytest.mark.anyio async def test_pin_session(self, client, make_session): session = await make_session("secretary") response = client.patch(f"/sessions/{session.id}/pin", json={"pinned": True}) diff --git a/tests/unit/core/test_agent.py b/tests/unit/core/test_agent.py index 3e70857..aa16acf 100644 --- a/tests/unit/core/test_agent.py +++ b/tests/unit/core/test_agent.py @@ -251,6 +251,31 @@ assert events[-1] == "StreamEnd" @pytest.mark.asyncio + async def test_run_stream_emits_todo_updated_after_tool_turn(self, agent, session): + """A TodoUpdated event is emitted after each tool-execution turn so the + UI side panel can reflect todo changes from the todo tool.""" + from navi.core.events import TodoUpdated + + backend = FakeLLMBackend( + responses=["", "final"], + tool_calls=[ + [ToolCallRequest(id="1", name="test_tool", arguments={})], + None, + ], + ) + agent._backends.register("ollama", backend) + + events = [] + async for ev in agent.run_stream(session.id, "do something"): + events.append(ev) + + todo_events = [ev for ev in events if isinstance(ev, TodoUpdated)] + assert len(todo_events) >= 1 + # The event carries the session id and a (possibly empty) tasks list. + assert todo_events[0].session_id == session.id + assert isinstance(todo_events[0].tasks, list) + + @pytest.mark.asyncio async def test_run_stream_stop_event(self, agent, session): """Cooperative stop mid-stream yields StreamStopped.""" from navi.tools._internal.base import current_stop_event diff --git a/tests/unit/core/test_events.py b/tests/unit/core/test_events.py index e023397..4871b37 100644 --- a/tests/unit/core/test_events.py +++ b/tests/unit/core/test_events.py @@ -1,7 +1,5 @@ """Unit tests for event dataclass wire serialization.""" -import pytest - from navi.core.events import ( AIHelperTokensUsed, CompressionStarted, @@ -16,6 +14,7 @@ TextDelta, ThinkingDelta, ThinkingEnd, + TodoUpdated, ToolEvent, ToolStarted, TurnThinking, @@ -168,3 +167,21 @@ ev = AIHelperTokensUsed(prompt_tokens=10, completion_tokens=20) assert ev.to_wire() is None assert ev.total == 30 + + +class TestTodoUpdated: + def test_to_wire_empty(self): + ev = TodoUpdated(session_id="sess1") + assert ev.to_wire() == {"type": "todo_updated", "session_id": "sess1", "tasks": []} + + def test_to_wire_with_tasks(self): + tasks = [ + {"index": 1, "text": "step one", "status": "done", "validation": "ran tests"}, + {"index": 2, "text": "step two", "status": "in_progress", "validation": ""}, + ] + ev = TodoUpdated(session_id="sess1", tasks=tasks) + wire = ev.to_wire() + assert wire["type"] == "todo_updated" + assert wire["session_id"] == "sess1" + assert wire["tasks"] == tasks + assert len(wire["tasks"]) == 2 diff --git a/tests/unit/core/test_planning.py b/tests/unit/core/test_planning.py index 90f5165..2f7c9e7 100644 --- a/tests/unit/core/test_planning.py +++ b/tests/unit/core/test_planning.py @@ -255,3 +255,51 @@ phase1_prompt = llm.calls[0][0].content assert "MODE: observe | act" in phase1_prompt assert "reading files to answer is still observe" in phase1_prompt + + +class _MemKv: + """Minimal in-memory KV store for the todo auto-populate isolation test.""" + + def __init__(self): + self._d: dict[tuple, str] = {} + + async def get(self, user_id, session_id, scope, key): + return self._d.get((user_id or "", session_id, scope, key)) + + async def set(self, user_id, session_id, scope, key, value): + self._d[(user_id or "", session_id, scope, key)] = value + + +class TestTodoIsolation: + async def test_auto_todo_lands_in_todo_session_row(self, monkeypatch): + """Planning auto-populates the todo into the row scoped by + current_todo_session_id (the sub-agent's run id), not the parent session id. + The parent's todo must stay empty so its goal-anchoring stays accurate.""" + from navi.tools._internal.base import ( + current_session_id, + current_todo_session_id, + ) + from navi.tools import todo as todo_mod + + kv = _MemKv() + monkeypatch.setattr(todo_mod, "_kv_store", kv) + + profile = make_profile("developer", planning_phase2_enabled=False) + llm = RecordingLLM( + [_ACT_ANALYSIS, "## Plan\n\n**Steps:**\n1. Build → TOOL: filesystem\n"] + ) + engine = PlanningEngine(FakeContextBuilder()) + context = [Message(role="user", content="build a thing")] + + t_tok = current_todo_session_id.set("sub_run_xyz") + s_tok = current_session_id.set("parent_sess") + try: + async for _event in engine.run(context, profile, llm, mem=None, tool_schemas=[]): + pass + finally: + current_todo_session_id.reset(t_tok) + current_session_id.reset(s_tok) + + # Sub-agent's KV row received the plan step; parent row is empty. + assert await kv.get(None, "sub_run_xyz", "todo", "tasks") is not None + assert await kv.get(None, "parent_sess", "todo", "tasks") is None diff --git a/tests/unit/tools/test_todo.py b/tests/unit/tools/test_todo.py index c271f89..32c96df 100644 --- a/tests/unit/tools/test_todo.py +++ b/tests/unit/tools/test_todo.py @@ -1,21 +1,18 @@ """Unit tests for navi.tools.todo.""" -import json - import pytest from navi.llm.base import Message from navi.tools.todo import ( TodoTool, - get_failed_steps, get_progress_message, get_task_snapshot, + load_tasks_for, set_tasks, - _kv_store, ) from navi.tools._internal.base import ToolContext from navi.store import KvStore -from tests.conftest_factory import FakeConnection, FakePool +from tests.conftest_factory import FakePool class FakeKvStore(KvStore): @@ -131,3 +128,181 @@ assert isinstance(msg, Message) assert msg.role == "system" assert "TODO progress" in msg.content + + +# ── Sub-agent todo isolation ────────────────────────────────────────────────── +# +# current_todo_session_id scopes a sub-agent's todo to its own ephemeral run id +# so its plan/updates do NOT clobber the parent session's todo (which the parent's +# goal-anchoring reads every iteration). These tests pin the _sid() precedence and +# the end-to-end isolation of the TodoTool writes. + + +@pytest.mark.asyncio +async def test_sid_prefers_todo_session_id_over_session_and_explicit(_fake_kv): + from navi.tools._internal.base import current_session_id, current_todo_session_id + + from navi.tools.todo import _sid + + t_tok = current_todo_session_id.set("sub_run_123") + s_tok = current_session_id.set("parent_sess") + try: + # todo-session-id wins over both current_session_id and an explicit arg. + assert _sid() == "sub_run_123" + assert _sid("explicit_sess") == "sub_run_123" + finally: + current_todo_session_id.reset(t_tok) + current_session_id.reset(s_tok) + + +@pytest.mark.asyncio +async def test_sid_falls_back_to_session_id_when_todo_unset(_fake_kv): + from navi.tools._internal.base import current_session_id + + from navi.tools.todo import _sid + + s_tok = current_session_id.set("parent_sess") + try: + assert _sid() == "parent_sess" + # Explicit arg still wins over the ContextVar fallback when todo-id is unset. + assert _sid("explicit_sess") == "explicit_sess" + finally: + current_session_id.reset(s_tok) + # And with everything unset, the default sentinel is used. + assert _sid() == "__default__" + + +@pytest.mark.asyncio +async def test_todo_tool_writes_isolated_subagent_row(_fake_kv): + """A sub-agent (current_todo_session_id set to its run id) writes its todo into + its own KV row, leaving the parent session's todo untouched.""" + from navi.tools._internal.base import current_session_id, current_todo_session_id + + tool = TodoTool() + # Parent session already has a todo plan. + await tool.execute( + {"op": "set", "tasks": ["parent task A"]}, + ctx=ToolContext(session_id="parent_sess", user_id="user1"), + ) + + # Sub-agent context: todo scoped to its run id; ctx.session_id is still the + # parent session id (other tools rely on it), but todo must isolate. + t_tok = current_todo_session_id.set("sub_run_abc") + s_tok = current_session_id.set("parent_sess") + try: + await tool.execute( + {"op": "set", "tasks": ["subagent step 1", "subagent step 2"]}, + ctx=ToolContext(session_id="parent_sess", user_id="user1"), + ) + await tool.execute( + {"op": "update", "index": 1, "status": "done", "validation": "verified"}, + ctx=ToolContext(session_id="parent_sess", user_id="user1"), + ) + finally: + current_todo_session_id.reset(t_tok) + current_session_id.reset(s_tok) + + # Sub-agent row carries its own tasks. + sub_snapshot = await get_task_snapshot("sub_run_abc") + assert sub_snapshot == frozenset( + {("subagent step 1", "done"), ("subagent step 2", "pending")} + ) + # Parent row is untouched — original single task, still pending. + parent_snapshot = await get_task_snapshot("parent_sess") + assert parent_snapshot == frozenset({("parent task A", "pending")}) + + +@pytest.mark.asyncio +async def test_progress_message_reads_isolated_rows(_fake_kv): + """get_progress_message is keyed by the sid it is given, so the parent's + goal-anchoring (which passes parent_sess) never sees the sub-agent's todo.""" + from navi.tools._internal.base import current_session_id, current_todo_session_id + + tool = TodoTool() + # Sub-agent writes a 2-step plan into its own row. + t_tok = current_todo_session_id.set("sub_run_def") + s_tok = current_session_id.set("parent_sess") + try: + await tool.execute( + {"op": "set", "tasks": ["sub step 1", "sub step 2"]}, + ctx=ToolContext(session_id="parent_sess", user_id="user1"), + ) + finally: + current_todo_session_id.reset(t_tok) + current_session_id.reset(s_tok) + + # Parent's progress message (keyed by parent_sess) sees no plan. + parent_msg = await get_progress_message("parent_sess") + assert parent_msg is None + # Sub-agent's row is readable by its own run id. + sub_msg = await get_progress_message("sub_run_def") + assert sub_msg is not None + assert "sub step 1" in sub_msg.content + + +# ── load_tasks_for: explicit-id reader for REST / TodoUpdated emitter ───────── + + +@pytest.mark.asyncio +async def test_load_tasks_for_returns_serialisable_dicts(_fake_kv): + await set_tasks("sess1", ["t1", "t2"]) + tasks = await load_tasks_for("sess1", None) + assert tasks == [ + {"index": 1, "text": "t1", "status": "pending", "validation": ""}, + {"index": 2, "text": "t2", "status": "pending", "validation": ""}, + ] + + +@pytest.mark.asyncio +async def test_load_tasks_for_empty_when_no_plan(_fake_kv): + assert await load_tasks_for("no-such-sess", None) == [] + + +@pytest.mark.asyncio +async def test_load_tasks_for_respects_user_id(_fake_kv): + """Rows are keyed by (user_id, session_id); load_tasks_for reads only the + requested user's row, not another user's for the same session id.""" + await _fake_kv.set( + "userA", "sess1", "todo", "tasks", + '[{"text": "A task", "status": "pending", "validation": ""}]', + ) + await _fake_kv.set( + "userB", "sess1", "todo", "tasks", + '[{"text": "B task", "status": "done", "validation": "x"}]', + ) + assert await load_tasks_for("sess1", "userA") == [ + {"index": 1, "text": "A task", "status": "pending", "validation": ""}, + ] + assert await load_tasks_for("sess1", "userB") == [ + {"index": 1, "text": "B task", "status": "done", "validation": "x"}, + ] + # No row for userC → empty. + assert await load_tasks_for("sess1", "userC") == [] + + +@pytest.mark.asyncio +async def test_load_tasks_for_uses_explicit_kv_not_global(_fake_kv): + """When an explicit kv is passed, the module-global _kv_store is ignored — + the REST route resolves its own KvStore via deps.""" + + class _MemKv: + def __init__(self): + self._d = {} + + async def get(self, user_id, session_id, scope, key): + return self._d.get((user_id or "", session_id, scope, key)) + + async def set(self, user_id, session_id, scope, key, value): + self._d[(user_id or "", session_id, scope, key)] = value + + # The autouse _fake_kv fixture backs the module global with a row. + await set_tasks("sess1", ["global-row"]) + # A separate in-memory kv with different contents must win when passed. + other = _MemKv() + await other.set(None, "sess1", "todo", "tasks", '[{"text": "kv-row", "status": "done", "validation": "x"}]') + tasks = await load_tasks_for("sess1", None, kv=other) + assert tasks == [{"index": 1, "text": "kv-row", "status": "done", "validation": "x"}] + # And the global (no kv arg) still reads the global row. + assert await load_tasks_for("sess1", None) == [ + {"index": 1, "text": "global-row", "status": "pending", "validation": ""}, + ]