diff --git a/clients/terminal/cli.py b/clients/terminal/cli.py index 2057a27..df8e975 100644 --- a/clients/terminal/cli.py +++ b/clients/terminal/cli.py @@ -4,6 +4,7 @@ import asyncio from pathlib import Path +from typing import TYPE_CHECKING import click @@ -13,6 +14,9 @@ from clients.terminal.state import StateManager from clients.terminal.ws_client import NaviWebSocketClient +if TYPE_CHECKING: + from clients.terminal.tui.tui_app import NaviCodeTui + @click.command( context_settings={ @@ -26,6 +30,12 @@ @click.option("--ws-url", default=None, help="Navi WebSocket URL (defaults to base-url).") @click.option("--profile-id", default=None, help="Profile to use for new sessions.") @click.option("--new-session", is_flag=True, help="Create a new session even if state exists.") +@click.option( + "--resume", + "resume_session_id", + default=None, + help="Resume a specific session by id (prints the id on close so you can re-run with it).", +) @click.option("--show-thinking", is_flag=True, help="Show model reasoning blocks.") @click.option("--show-events/--no-events", default=True, help="Show tool call events.") @click.option("--theme", default=None, help="TUI theme name to start with.") @@ -43,6 +53,7 @@ theme: str | None, mouse: bool | None, raw: bool, + resume_session_id: str | None, ) -> None: """Navi Code — terminal client for Navi. @@ -57,18 +68,27 @@ settings.show_thinking = True settings.show_events = show_events + if resume_session_id and new_session: + raise click.ClickException("--resume and --new-session are mutually exclusive.") + cwd = Path.cwd().resolve() if raw or prompt: - _run_raw(prompt, new_session, profile_id, cwd) + _run_raw(prompt, new_session, profile_id, cwd, resume_session_id) return - _run_tui(profile_id, new_session, theme, mouse, cwd) + _run_tui(profile_id, new_session, theme, mouse, cwd, resume_session_id) -def _run_raw(prompt: str | None, new_session: bool, profile_id: str | None, cwd: Path) -> None: +def _run_raw( + prompt: str | None, + new_session: bool, + profile_id: str | None, + cwd: Path, + resume_session_id: str | None, +) -> None: state = StateManager() - session_id = _resolve_session_id(state, new_session, profile_id) + session_id = _resolve_session_id(state, new_session, profile_id, resume_session_id) if not session_id: raise click.ClickException("Failed to create or resume a session.") @@ -88,16 +108,59 @@ theme: str | None, mouse: bool | None, cwd: Path, + resume_session_id: str | None, ) -> None: from clients.terminal.tui.tui_app import NaviCodeTui - app = NaviCodeTui(profile_id=profile_id, new_session=new_session, theme_name=theme, cwd=cwd) + app = NaviCodeTui( + profile_id=profile_id, + new_session=new_session, + session_id=resume_session_id, + theme_name=theme, + cwd=cwd, + ) if mouse is not None: app._mouse_enabled = mouse app.run(mouse=app._mouse_enabled) + _print_resume_hint(app) -def _resolve_session_id(state: StateManager, force_new: bool, profile_id: str | None) -> str | None: +def _resume_hint(session_id: str) -> str: + """The exact command a user can re-run to continue this session.""" + return f"navi-code --resume {session_id}" + + +def _print_resume_hint(app: NaviCodeTui) -> None: + """After the TUI exits, tell the user how to resume the session they had open. + + Printed here (not from on_unmount) because Textual has already restored the + terminal by the time app.run() returns, so the hint lands on clean stdout. + """ + session_id = getattr(app._ctx, "session_id", None) + if not session_id: + return + click.secho(f"Session {session_id}", fg="bright_black") + click.secho(f"Resume with: {_resume_hint(session_id)}", fg="cyan") + + +def _resolve_session_id( + state: StateManager, + force_new: bool, + profile_id: str | None, + resume_session_id: str | None = None, +) -> str | None: + if resume_session_id: + try: + session = api.get_session(resume_session_id) + except Exception as exc: + raise click.ClickException(f"Failed to resume session {resume_session_id[:8]}: {exc}") + state.set_session_id(session["session_id"]) + click.secho( + f"Resumed session {session['session_id'][:8]} (profile {session['profile_id']})", + fg="bright_black", + ) + return session["session_id"] + if not force_new: saved = state.get_session_id() if saved: diff --git a/clients/terminal/tui/tui_app.py b/clients/terminal/tui/tui_app.py index c26d433..3c7bd2c 100644 --- a/clients/terminal/tui/tui_app.py +++ b/clients/terminal/tui/tui_app.py @@ -132,11 +132,18 @@ force_new: bool, ) -> str | None: if session_id and not force_new: + # An explicitly requested id (the --resume flag) must not silently + # fall through to creating a new session on a typo — surface the + # error and let the user correct the id. Saved-state resume below + # is the lenient path. try: session = api.get_session(session_id) return session["session_id"] - except Exception: - pass + except Exception as exc: + self._chat_panel.handle_ws_event( + {"type": "error", "message": f"Failed to resume session {session_id[:8]}: {exc}"} + ) + return None if not force_new: saved = self._state.get_session_id() diff --git a/tests/clients/test_terminal_client.py b/tests/clients/test_terminal_client.py index c6ae206..7d7b3dd 100644 --- a/tests/clients/test_terminal_client.py +++ b/tests/clients/test_terminal_client.py @@ -8,7 +8,7 @@ import pytest from click.testing import CliRunner -from clients.terminal.cli import main +from clients.terminal.cli import main, _resolve_session_id, _resume_hint from clients.terminal.config import Settings from clients.terminal.render import Renderer from clients.terminal.state import StateManager @@ -144,3 +144,62 @@ assert "Resumed session sess-raw" in result.output finally: config.settings.state_dir = original_state_dir + + def test_resume_hint_format(self) -> None: + assert _resume_hint("sess-123-abc") == "navi-code --resume sess-123-abc" + + def test_resume_flag_resumes_given_session( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """--resume resolves that exact session and persists it to state.""" + import clients.terminal.api as api_module + + monkeypatch.setattr(api_module, "get_session", lambda sid: { + "session_id": sid, + "profile_id": "navi_code", + "name": "Resumed", + }) + + from clients.terminal import config + + original_state_dir = config.settings.state_dir + config.settings.state_dir = tmp_path + try: + state = StateManager(tmp_path) + session_id = _resolve_session_id(state, force_new=False, profile_id=None, resume_session_id="sess-resume-xyz") + assert session_id == "sess-resume-xyz" + assert state.get_session_id() == "sess-resume-xyz" + finally: + config.settings.state_dir = original_state_dir + + def test_resume_flag_with_bad_session_errors( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """A --resume id that the server can't find is a hard error, not a silent new session.""" + import clients.terminal.api as api_module + + def boom(_sid: str) -> dict: + raise RuntimeError("session not found") + + monkeypatch.setattr(api_module, "get_session", boom) + monkeypatch.setattr(api_module, "create_session", lambda *_a, **_k: {"session_id": "NEW", "profile_id": "navi_code"}) + + import click + state = StateManager(tmp_path) + with pytest.raises(click.ClickException, match="Failed to resume"): + _resolve_session_id(state, force_new=False, profile_id=None, resume_session_id="sess-bad") + # Must NOT have created/persisted a new session behind the user's back. + assert state.get_session_id() is None + + def test_resume_and_new_session_are_mutually_exclusive( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + import clients.terminal.api as api_module + + monkeypatch.setattr(api_module, "create_session", lambda *_a, **_k: {"session_id": "x", "profile_id": "navi_code"}) + monkeypatch.setattr(api_module, "get_session", lambda sid: {"session_id": sid, "profile_id": "navi_code"}) + + runner = CliRunner() + result = runner.invoke(main, ["--resume", "sess-x", "--new-session"]) + assert result.exit_code != 0 + assert "mutually exclusive" in result.output diff --git a/tests/clients/test_tui_app.py b/tests/clients/test_tui_app.py index 6b8982d..7f00d50 100644 --- a/tests/clients/test_tui_app.py +++ b/tests/clients/test_tui_app.py @@ -450,3 +450,55 @@ await pilot.press("escape") await pilot.pause(0.1) assert calls == [] + + +@pytest.mark.anyio +async def test_resolve_session_with_explicit_id_resumes_it(monkeypatch: pytest.MonkeyPatch) -> None: + """An explicit session_id (--resume) resumes that exact session.""" + import clients.terminal.api as api_module + + seen: list[str] = [] + monkeypatch.setattr(api_module, "get_session", lambda sid: seen.append(sid) or { + "session_id": sid, + "profile_id": "navi_code", + }) + + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + # _startup already attached a session (calling get_session once), so + # snapshot the recorder baseline before our explicit resume call. + baseline = len(seen) + result = await pilot.app._resolve_session("sess-explicit", None, False) + assert result == "sess-explicit" + assert seen[baseline:] == ["sess-explicit"] + + +@pytest.mark.anyio +async def test_resolve_session_strict_on_bad_explicit_id(monkeypatch: pytest.MonkeyPatch) -> None: + """A bad --resume id surfaces an error and does NOT silently create a new session.""" + import clients.terminal.api as api_module + + def boom(_sid: str) -> dict: + raise RuntimeError("session not found") + + created: list[str] = [] + monkeypatch.setattr(api_module, "get_session", boom) + monkeypatch.setattr(api_module, "create_session", lambda *_a, **_k: created.append("x") or { + "session_id": "NEW", + "profile_id": "navi_code", + }) + + async with NaviCodeTui(new_session=True).run_test() as pilot: + await pilot.pause() + # _startup (force_new=True) already created a session; snapshot so we + # only assert about what our bad-resume call does. + created_before = len(created) + chat = pilot.app.query_one("ChatPanel") + before_errors = sum(1 for it in chat._model.items if it.kind == "error") + result = await pilot.app._resolve_session("sess-bad", None, False) + assert result is None + # No new session was created behind the user's back. + assert len(created) == created_before + # An error was surfaced to the chat panel. + after_errors = sum(1 for it in chat._model.items if it.kind == "error") + assert after_errors == before_errors + 1