From 47a501074c6c2d1714029bd1aa5690231534e493 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Mon, 21 Sep 2026 01:06:37 -0700 Subject: [PATCH 01/11] fix(web): open a saved session in milliseconds, and start one in a new workspace or worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a sidebar row took ~45 s on this repo. The click waited on session.resume, which spawns a runtime, and the runtime's system prompt walked the whole workspace with Path.rglob (node_modules included: 366k directories, ~23 s) — twice per resume, once at spawn and once when the stored conversation was loaded. The sidebar tree then re-read every saved session file and re-probed git for every distinct cwd (~3.7 s) after each navigation. Server - workspace_snapshot: one bounded scandir walk that prunes vendored and generated directories before entering them and stops at a directory and entry budget; partial counts render as "N+ (partial scan)". 23 s → 0.1 s. - session.history: a saved transcript read cold, no runtime touched. - session.resume reuses the runtime already replaying a row (state.sessions is keyed by runtime id, so a row's id never matched) and a concurrent resume of the same row waits on the in-flight attach instead of spawning. - session.create takes create_dir (mkdir -p a new workspace) and worktree (run in a fresh .clawcodex/worktrees/ checkout, the CLI's --worktree). - projects.tree: sidebar rows cached on (mtime, size); git probes memoized across rebuilds with a TTL and run in parallel when cold. 3.7 s → 0.1 s. Web client - A row click renders session.history at once (title, workspace, model, nodes, trajectory), then attaches the runtime behind it; the composer says "Connecting the agent…" and a prompt sent meanwhile waits for the attach. - Leaving a session releases its runtime when idle (no turn, no pending approval or question, nothing queued), so browsing does not accumulate agents. Older backends without session.history get the one-call resume. - New session opens a dialog: pick a known workspace or "Create new workspace…" with an absolute folder path, and a Worktree switch to isolate the session in its own git worktree. Refusals stay in the dialog. Co-Authored-By: Claude Fable 5.1 --- src/context_system/builder.py | 9 +- src/context_system/models.py | 3 + src/context_system/workspace_snapshot.py | 99 ++++++- src/server/desktop_gateway_methods.py | 243 ++++++++++++++++-- src/server/desktop_projects.py | 77 +++++- src/server/desktop_serve.py | 8 + src/server/desktop_sessions.py | 76 +++++- tests/server/test_desktop_gateway.py | 196 ++++++++++++++ tests/server/test_desktop_projects.py | 36 +++ tests/server/test_desktop_sessions.py | 58 +++++ tests/test_workspace_snapshot.py | 118 +++++++++ ui-web/README.md | 47 +++- ui-web/src/App.tsx | 10 +- ui-web/src/conversation/InputBar.tsx | 13 +- ui-web/src/gateway/protocol.ts | 30 +++ .../src/sidebar/NewSessionDialog.module.css | 171 ++++++++++++ ui-web/src/sidebar/NewSessionDialog.test.tsx | 129 ++++++++++ ui-web/src/sidebar/NewSessionDialog.tsx | 213 +++++++++++++++ ui-web/src/sidebar/Sidebar.test.tsx | 13 + ui-web/src/sidebar/Sidebar.tsx | 11 +- ui-web/src/state/actions.test.ts | 233 +++++++++++++++++ ui-web/src/state/actions.ts | 214 ++++++++++++--- ui-web/src/state/store.ts | 13 + 23 files changed, 1922 insertions(+), 98 deletions(-) create mode 100644 tests/test_workspace_snapshot.py create mode 100644 ui-web/src/sidebar/NewSessionDialog.module.css create mode 100644 ui-web/src/sidebar/NewSessionDialog.test.tsx create mode 100644 ui-web/src/sidebar/NewSessionDialog.tsx diff --git a/src/context_system/builder.py b/src/context_system/builder.py index 1eaa55d6d..7501df1cd 100644 --- a/src/context_system/builder.py +++ b/src/context_system/builder.py @@ -101,8 +101,8 @@ def _build_workspace_section(root: Path, current: Path) -> str: f"- Today's date: {date.today().isoformat()}", f"- Workspace root: {workspace.workspace_root}", f"- Current directory: {workspace.current_directory}", - f"- Python files: {workspace.python_file_count}", - f"- Test files: {workspace.test_file_count}", + f"- Python files: {_count_text(workspace.python_file_count, workspace.counts_partial)}", + f"- Test files: {_count_text(workspace.test_file_count, workspace.counts_partial)}", ] if workspace.key_files: lines.append(f"- Key files: {', '.join(workspace.key_files)}") @@ -111,6 +111,11 @@ def _build_workspace_section(root: Path, current: Path) -> str: return "\n".join(lines) +def _count_text(count: int, partial: bool) -> str: + """``1234``, or ``1234+ (partial scan)`` when the walk hit its budget.""" + return f"{count}+ (partial scan)" if partial else str(count) + + def _build_git_section(cwd: str) -> str: from .git_context import collect_git_context, format_git_status try: diff --git a/src/context_system/models.py b/src/context_system/models.py index 27b70f33a..79d0e8a96 100644 --- a/src/context_system/models.py +++ b/src/context_system/models.py @@ -105,3 +105,6 @@ class WorkspaceSnapshot: key_files: tuple[str, ...] python_file_count: int test_file_count: int + #: True when the file walk hit its directory budget, so the counts are + #: lower bounds — rendered as ``N+ (partial scan)``. + counts_partial: bool = False diff --git a/src/context_system/workspace_snapshot.py b/src/context_system/workspace_snapshot.py index e3ff6caf7..a46940b71 100644 --- a/src/context_system/workspace_snapshot.py +++ b/src/context_system/workspace_snapshot.py @@ -1,18 +1,61 @@ +"""The ``## Runtime Context`` facts about a workspace: shape, key files, counts. + +The Python/test file counts come from ONE bounded ``os.scandir`` walk that +never descends into vendored or generated directories (``node_modules``, +``.git``, virtualenvs, caches, ``.clawcodex`` worktree checkouts …) and stops +after :data:`MAX_SCANNED_DIRS` directories. The previous implementation used +``Path.rglob`` twice, which walks *everything* and filters afterwards: on a +workspace with a few front-end packages (hundreds of thousands of +``node_modules`` directories) each walk took ~20 s, and the system prompt is +built at spawn and again on every resume/clear — so opening a saved session +from the web sidebar sat on this for ~45 s. The counts are a hint for the +model, not an inventory: a bounded scan that says "N+ (partial)" is the right +trade, and it keeps the cost proportional to the project rather than to +whatever a package manager left on disk. +""" + from __future__ import annotations +import os +from collections import deque from pathlib import Path from .models import WorkspaceSnapshot -_IGNORED_NAMES = { +#: Directory names the walk never enters. Vendored trees, VCS internals, +#: virtualenvs, tool caches, build output and the per-repo ``.clawcodex`` +#: directory (its ``worktrees/`` holds whole extra checkouts of the repo, +#: which would count every file again per worktree). +_IGNORED_NAMES = frozenset({ ".git", + ".hg", + ".svn", ".venv", + "venv", + ".tox", + ".nox", + ".eggs", + ".cache", ".pytest_cache", ".mypy_cache", ".ruff_cache", "__pycache__", "node_modules", -} + ".clawcodex", + ".claude", + "site-packages", +}) + +#: Upper bound on directories one snapshot visits (after pruning). Roughly a +#: few tens of milliseconds on a warm disk; past it the counts are reported as +#: partial rather than the walk running on. +MAX_SCANNED_DIRS = 2500 + +#: Upper bound on directory entries one snapshot reads, so a single flat +#: directory of a million files (a dataset dump next to the code) is bounded +#: the same way a deep tree is. +MAX_SCANNED_ENTRIES = 200_000 + _KEY_FILE_CANDIDATES = ( "README.md", "CLAWCODEX.md", @@ -29,6 +72,7 @@ def build_workspace_snapshot( *, cwd: str | Path | None = None, top_level_limit: int = 12, + max_dirs: int = MAX_SCANNED_DIRS, ) -> WorkspaceSnapshot: root = Path(workspace_root).expanduser().resolve() current = Path(cwd).expanduser().resolve() if cwd is not None else root @@ -49,8 +93,7 @@ def build_workspace_snapshot( break key_files = tuple(name for name in _KEY_FILE_CANDIDATES if (root / name).exists()) - python_file_count = sum(1 for path in root.rglob("*.py") if _is_countable(path)) - test_file_count = sum(1 for path in root.rglob("test_*.py") if _is_countable(path)) + python_file_count, test_file_count, partial = count_python_files(root, max_dirs=max_dirs) return WorkspaceSnapshot( workspace_root=root, @@ -59,9 +102,54 @@ def build_workspace_snapshot( key_files=key_files, python_file_count=python_file_count, test_file_count=test_file_count, + counts_partial=partial, ) +def count_python_files(root: Path, *, max_dirs: int = MAX_SCANNED_DIRS) -> tuple[int, int, bool]: + """``(python_files, test_files, partial)`` under ``root``, pruned and bounded. + + One breadth-first ``scandir`` walk: ignored directory names are skipped + *before* they are entered (this is what makes the walk cheap — filtering + ``rglob`` output afterwards still pays for every directory it crawled), + symlinks are never followed, and once ``max_dirs`` directories have been + scanned the walk stops and ``partial`` is True. Breadth-first so a partial + scan still covers the project's shallow structure rather than one deep + corner of it. Entries are visited in name order, so a partial count is + deterministic for a given tree. + """ + python_files = 0 + test_files = 0 + scanned = 0 + entries_seen = 0 + pending: deque[str] = deque([str(root)]) + while pending: + if scanned >= max_dirs or entries_seen >= MAX_SCANNED_ENTRIES: + return python_files, test_files, True + directory = pending.popleft() + scanned += 1 + try: + with os.scandir(directory) as scan: + items = sorted(scan, key=lambda entry: entry.name) + except OSError: + continue + entries_seen += len(items) + for entry in items: + name = entry.name + try: + if entry.is_dir(follow_symlinks=False): + if name not in _IGNORED_NAMES: + pending.append(entry.path) + continue + if name.endswith(".py") and entry.is_file(follow_symlinks=False): + python_files += 1 + if name.startswith("test_"): + test_files += 1 + except OSError: + continue + return python_files, test_files, False + + def _is_within(child: Path, parent: Path) -> bool: try: child.relative_to(parent) @@ -70,5 +158,4 @@ def _is_within(child: Path, parent: Path) -> bool: return False -def _is_countable(path: Path) -> bool: - return path.is_file() and not any(part in _IGNORED_NAMES for part in path.parts) +__all__ = ["MAX_SCANNED_DIRS", "MAX_SCANNED_ENTRIES", "build_workspace_snapshot", "count_python_files"] diff --git a/src/server/desktop_gateway_methods.py b/src/server/desktop_gateway_methods.py index d11c703b0..6662995d0 100644 --- a/src/server/desktop_gateway_methods.py +++ b/src/server/desktop_gateway_methods.py @@ -142,6 +142,17 @@ def __init__(self, session_id: str, state: DesktopServeState) -> None: self.pump_task: asyncio.Task | None = None self.init_info: dict[str, Any] = {} self.init_seen = asyncio.Event() + # The saved session this runtime replays (``session.resume``), or None + # for one created fresh. ``state.sessions`` is keyed by RUNTIME id and + # a resumed row gets a fresh runtime, so this is how a second click on + # the same row finds the runtime it already has instead of spawning + # another. + self.stored_id: str | None = None + # Set once ``_create`` has finished attaching this runtime (spawned, + # capability-negotiated, stored conversation loaded) — or given up. + # A concurrent resume of the same row waits on it rather than racing + # a second spawn. + self.attached = asyncio.Event() # My queries INTO the agent (control_request → control_response). self._pending_control: dict[str, asyncio.Future] = {} # The agent's asks OF the user (can_use_tool …), keyed by request_id; @@ -955,6 +966,49 @@ def _clean(value: Any) -> str | None: return None +def _prepare_workspace(path: str, create: bool) -> str: + """An absolute directory a session can run in, created when asked. + + Raises ``ValueError`` — the gateway's "this is the caller's mistake" error + — for a relative path, a path that is a file, or a folder that does not + exist when ``create`` is off. Runs off the event loop (``makedirs`` and + the stats can block on a slow volume). + """ + import os + + expanded = os.path.expanduser(path.strip()) + if not os.path.isabs(expanded): + raise ValueError(f"workspace path must be absolute: {path}") + normalized = os.path.normpath(expanded) + if os.path.isdir(normalized): + return normalized + if os.path.exists(normalized): + raise ValueError(f"not a directory: {normalized}") + if not create: + raise ValueError(f"no such directory: {normalized}") + try: + os.makedirs(normalized, exist_ok=True) + except OSError as exc: + raise ValueError(f"cannot create {normalized}: {exc.strerror or exc}") from exc + return normalized + + +def _create_session_worktree(cwd: str) -> dict[str, Any]: + """A fresh worktree of the repo at ``cwd`` (the CLI's bare ``--worktree``).""" + from src.utils.worktree_session import WorktreeError, create_worktree_for_session + + try: + session = create_worktree_for_session(None, cwd=cwd) + except WorktreeError as exc: + raise ValueError(str(exc)) from exc + return { + "name": session.worktree_name, + "path": session.worktree_path, + "branch": session.worktree_branch, + "repo_root": session.repo_root, + } + + def _positive_int(value: Any, fallback: int) -> int: """A positive integer from a JSON field, or ``fallback``. @@ -986,6 +1040,7 @@ def __init__(self, websocket: WebSocket, state: DesktopServeState) -> None: self.method_handlers = { "session.create": self.session_create, "session.resume": self.session_resume, + "session.history": self.session_history, "session.activate": self.session_activate, "session.close": self.session_close, "session.active_list": self.session_active_list, @@ -1062,17 +1117,46 @@ def _session(self, params: dict[str, Any]) -> DesktopSession: raise ValueError(f"session {session_id} is still starting") return session + def _live_session_for(self, stored_id: str) -> DesktopSession | None: + """The runtime this server already has for ``stored_id``, if any. + + Either the runtime itself (a session created here saves under its own + id) or the fresh runtime a resume of that row spawned (``stored_id``). + Without the second match every click on a row the server had already + replayed spawned yet another runtime and left the previous one alive. + """ + direct = self.state.sessions.get(stored_id) + if direct is not None: + return direct + for session in self.state.sessions.values(): + if session.stored_id == stored_id: + return session + return None + async def _create(self, cwd: str | None, resume: str | None, params: dict[str, Any] | None = None) -> DesktopSession: manager = self.state.manager workspace = cwd or self.state.workspace - if resume and resume in self.state.sessions: - return self.state.sessions[resume] + if resume: + existing = self._live_session_for(resume) + if existing is not None: + # A second resume while the first is still attaching waits + # for it — two rapid clicks must not become two runtimes. + if not existing.attached.is_set(): + try: + await asyncio.wait_for(existing.attached.wait(), CONTROL_TIMEOUT_S) + except asyncio.TimeoutError: + pass + # Still registered: the attach succeeded (or is stuck past + # its timeout, in which case a second spawn would not help). + if existing.session_id in self.state.sessions: + return existing # A resumed stored session still gets a fresh runtime session: spawn, # then load the stored conversation via the `resume` control below. info = manager.create_session(cwd=workspace) session_id = info.id session = DesktopSession(session_id, self.state) + session.stored_id = resume session.sockets.add(self.websocket) self.state.sessions[session_id] = session # Honor the composer's provider/model/effort selection at spawn time, so @@ -1109,7 +1193,19 @@ async def _create(self, cwd: str | None, resume: str | None, # sessionless call (model.options, commands.catalog …) from any # window picks it up. Re-raised untouched; this only cleans up. self.state.sessions.pop(session_id, None) + session.attached.set() raise + try: + await self._attach(session, resume, params) + finally: + session.attached.set() + return session + + async def _attach(self, session: DesktopSession, resume: str | None, + params: dict[str, Any]) -> None: + """Finish a freshly spawned runtime: init, capabilities, stored replay.""" + manager = self.state.manager + session_id = session.session_id try: manager.mark_running(session_id) except Exception: # noqa: BLE001 — index upkeep is best-effort @@ -1140,7 +1236,6 @@ async def _create(self, cwd: str | None, resume: str | None, if not isinstance(reply, dict) or reply.get("ok") is False: logger.warning("session %s: resume of %s refused: %r", session_id, resume, reply) - return session # ── methods ────────────────────────────────────────────────────────────── @@ -1153,6 +1248,9 @@ async def projects_tree(self, params: dict[str, Any]) -> dict[str, Any]: return await _asyncio.to_thread(self._build_projects_tree, preview_limit) def _build_projects_tree(self, preview_limit: int) -> dict[str, Any]: + import os + from concurrent.futures import ThreadPoolExecutor + from src.server.desktop_projects import build_project_tree, canonical_workspace_path from src.server.desktop_sessions import list_session_rows from src.utils.git import get_repo_root, list_worktrees @@ -1177,19 +1275,38 @@ def _build_projects_tree(self, preview_limit: int) -> dict[str, Any]: }) seen.add(sid) - # Per-cwd / per-repo memoized git probes: a tree can hold many sessions - # in the same repo, so probe each distinct path once. - repo_cache: dict[str, str | None] = {} - wt_cache: dict[str, list[str]] = {} + # Git probes are memoized ACROSS rebuilds on the serve state: the + # tree is rebuilt after every turn end and every session switch, and + # a sessions dir accumulates thousands of distinct cwds, so probing + # each one every time cost seconds per rebuild. + cache = self.state.probe_cache workspace_cache: dict[str, str | None] = {} def worktrees_of(repo_root: str) -> list[str]: # ``git worktree list`` is repo-global and main-first from ANY # worktree in the repo, so this is correct whether keyed by the # main root or a linked-worktree path. - if repo_root not in wt_cache: - wt_cache[repo_root] = [w.path for w in list_worktrees(repo_root) if w.path] - return wt_cache[repo_root] + cached = cache.worktrees(repo_root) + if cached is None: + cached = [w.path for w in list_worktrees(repo_root) if w.path] + cache.set_worktrees(repo_root, cached) + return cached + + def probe_toplevel(cwd: str) -> str | None: + # A cwd that is gone — a deleted temp dir, an unmounted volume — + # is not a repo, and needs no git call to say so. + if not os.path.isdir(cwd): + return None + return get_repo_root(cwd) or None + + # Every cwd the cache cannot answer, probed in parallel: each is one + # subprocess, and the first tree after boot has all of them to do. + wanted = {c for c in (str(r.get("cwd") or "").strip() for r in rows) if c} + missing = [c for c in wanted if not cache.has_repo_root(c)] + if missing: + with ThreadPoolExecutor(max_workers=min(8, len(missing))) as pool: + for cwd, top in zip(missing, pool.map(probe_toplevel, missing)): + cache.set_repo_root(cwd, top) def repo_root_of(cwd: str) -> str | None: # ``rev-parse --show-toplevel`` inside a LINKED worktree returns the @@ -1197,14 +1314,13 @@ def repo_root_of(cwd: str) -> str | None: # worktree into its own project. Resolve the MAIN worktree root # (the first ``git worktree list`` entry) so linked worktrees group # as lanes under their repo, matching the renderer's tree. - if cwd not in repo_cache: - top = get_repo_root(cwd) - if top: - worktrees = worktrees_of(top) - repo_cache[cwd] = worktrees[0] if worktrees else top - else: - repo_cache[cwd] = None - return repo_cache[cwd] + if not cache.has_repo_root(cwd): + cache.set_repo_root(cwd, probe_toplevel(cwd)) + top = cache.repo_root(cwd) + if not top: + return None + worktrees = worktrees_of(top) + return worktrees[0] if worktrees else top def workspace_path_of(cwd: str) -> str | None: if cwd not in workspace_cache: @@ -1221,12 +1337,90 @@ def workspace_path_of(cwd: str) -> str | None: ) async def session_create(self, params: dict[str, Any]) -> dict[str, Any]: - session = await self._create(params.get("cwd"), None, params) - return { + """Spawn a fresh session. + + ``cwd`` names the workspace; with ``create_dir`` a folder that does + not exist yet is created (the "new workspace" flow, mkdir -p). With + ``worktree`` the session runs in a fresh git worktree of that repo + (``.clawcodex/worktrees/``, the CLI's ``--worktree``), which is + left in place when the session ends — a browser tab has no exit + dialog to offer keep-or-remove. Both are validated here so a bad path + is an error with a name rather than a runtime that fails to start. + """ + cwd = _clean(params.get("cwd")) + if cwd is not None: + cwd = await asyncio.to_thread( + _prepare_workspace, cwd, bool(params.get("create_dir")) + ) + worktree: dict[str, Any] | None = None + if params.get("worktree") is True: + worktree = await asyncio.to_thread( + _create_session_worktree, cwd or self.state.workspace + ) + cwd = worktree["path"] + # The next sidebar tree must show the new lane. + self.state.probe_cache.forget_worktrees(worktree["repo_root"]) + session = await self._create(cwd, None, params) + reply: dict[str, Any] = { "session_id": session.session_id, "stored_session_id": session.session_id, "info": _init_session_info(session.init_info), } + if worktree is not None: + reply["worktree"] = worktree + return reply + + async def session_history(self, params: dict[str, Any]) -> dict[str, Any]: + """A saved session's transcript, cold: no runtime is spawned or touched. + + What the web client renders the moment a sidebar row is clicked; the + runtime attaches afterwards (``session.resume``) without the reader + waiting on it. Same message shape as ``session.resume`` returns. + + When this server already has a runtime replaying the row, its OWN + record is preferred — it holds the turns run since the row was + resumed, which the row's file never learns about. + """ + wanted = str(params.get("session_id") or "") + if not wanted: + raise ValueError("session_id required") + from src.server.desktop_sessions import load_session_messages + + sessions_dir = self.state.saved_sessions_dir() + live = self._live_session_for(wanted) + stored = None + if live is not None and live.session_id != wanted: + stored = await asyncio.to_thread(load_session_messages, sessions_dir, live.session_id) + if stored is None: + stored = await asyncio.to_thread(load_session_messages, sessions_dir, wanted) + if stored is None: + if live is None: + raise ValueError(f"unknown session: {wanted}") + # A runtime that never saved (nothing typed since it was made): + # there is nothing to show, and that is not an error. + return { + "session_id": wanted, + "stored_session_id": wanted, + "live_session_id": live.session_id, + "found": False, + "messages": [], + "message_count": 0, + "info": _init_session_info(live.init_info), + } + info = {key: stored[key] for key in ("cwd", "model", "provider") if stored.get(key)} + reply: dict[str, Any] = { + "session_id": wanted, + "stored_session_id": wanted, + "found": True, + "messages": stored["messages"], + "message_count": stored["message_count"], + "info": info, + } + if stored.get("title"): + reply["title"] = stored["title"] + if live is not None: + reply["live_session_id"] = live.session_id + return reply async def session_resume(self, params: dict[str, Any]) -> dict[str, Any]: wanted = str(params.get("session_id") or "") or None @@ -1257,7 +1451,14 @@ async def session_resume(self, params: dict[str, Any]) -> dict[str, Any]: if wanted and not omit: from src.server.desktop_sessions import load_session_messages - stored = load_session_messages(self.state.saved_sessions_dir(), wanted) + sessions_dir = self.state.saved_sessions_dir() + stored = None + # A runtime already replaying this row has the complete record + # (see session_history); the row's own file is the fallback. + if session.session_id != wanted: + stored = await asyncio.to_thread(load_session_messages, sessions_dir, session.session_id) + if stored is None: + stored = await asyncio.to_thread(load_session_messages, sessions_dir, wanted) if stored is not None: response["messages"] = stored["messages"] response["message_count"] = stored["message_count"] diff --git a/src/server/desktop_projects.py b/src/server/desktop_projects.py index 0ead2e0e0..582b0e0e5 100644 --- a/src/server/desktop_projects.py +++ b/src/server/desktop_projects.py @@ -31,11 +31,86 @@ from __future__ import annotations import os +import threading +import time from typing import Any, Callable NO_PROJECT_ID = "__no_project__" +class ProbeCache: + """Memoized git probes for the sidebar tree, shared across rebuilds. + + ``projects.tree`` shells out once per distinct session cwd (``git + rev-parse --show-toplevel``) and once per repo (``git worktree list``). A + sessions directory accumulates thousands of distinct cwds over time — + temp dirs, worktrees, test fixtures — so probing them all on every rebuild + cost seconds, and the tree is rebuilt after every turn end and every + session switch. A repo root answer is kept for ``ttl_s`` (a directory's + repo does not move); the worktree list for ``worktree_ttl_s``, and + :meth:`forget_worktrees` drops it early when this server adds one. + Thread-safe: the tree is built off the event loop and probes run in a + pool. + """ + + def __init__( + self, + *, + ttl_s: float = 300.0, + worktree_ttl_s: float = 30.0, + clock: Callable[[], float] = time.monotonic, + ) -> None: + self._ttl_s = ttl_s + self._worktree_ttl_s = worktree_ttl_s + self._clock = clock + self._lock = threading.Lock() + # cwd → (expires_at, toplevel or None) + self._repo_root: dict[str, tuple[float, str | None]] = {} + # repo root → (expires_at, worktree paths, main first) + self._worktrees: dict[str, tuple[float, list[str]]] = {} + + def has_repo_root(self, cwd: str) -> bool: + with self._lock: + entry = self._repo_root.get(cwd) + return entry is not None and entry[0] > self._clock() + + def repo_root(self, cwd: str) -> str | None: + """The cached toplevel for ``cwd`` (None: not a repo, or not cached).""" + with self._lock: + entry = self._repo_root.get(cwd) + if entry is None or entry[0] <= self._clock(): + return None + return entry[1] + + def set_repo_root(self, cwd: str, toplevel: str | None) -> None: + with self._lock: + self._repo_root[cwd] = (self._clock() + self._ttl_s, toplevel) + + def worktrees(self, repo_root: str) -> list[str] | None: + with self._lock: + entry = self._worktrees.get(repo_root) + if entry is None or entry[0] <= self._clock(): + return None + return list(entry[1]) + + def set_worktrees(self, repo_root: str, paths: list[str]) -> None: + with self._lock: + self._worktrees[repo_root] = (self._clock() + self._worktree_ttl_s, list(paths)) + + def forget_worktrees(self, repo_root: str | None = None) -> None: + """Drop the worktree list for one repo, or every repo's.""" + with self._lock: + if repo_root is None: + self._worktrees.clear() + else: + self._worktrees.pop(repo_root, None) + + def clear(self) -> None: + with self._lock: + self._repo_root.clear() + self._worktrees.clear() + + def canonical_workspace_path(path: str) -> str | None: """Return the real path for an existing directory, else ``None``. @@ -262,4 +337,4 @@ def _lane_for(repo_root: str, cwd: str) -> tuple[str, str, bool, str]: } -__all__ = ["build_project_tree", "canonical_workspace_path", "NO_PROJECT_ID"] +__all__ = ["ProbeCache", "build_project_tree", "canonical_workspace_path", "NO_PROJECT_ID"] diff --git a/src/server/desktop_serve.py b/src/server/desktop_serve.py index 101363426..e9b11b662 100644 --- a/src/server/desktop_serve.py +++ b/src/server/desktop_serve.py @@ -65,6 +65,8 @@ class DesktopServeState: sessions: dict[str, Any] = field(default_factory=dict) # Saved-transcript dir override (tests); default resolves per request. sessions_dir: Path | None = None + # Git probe answers the sidebar tree reuses across rebuilds. + probe_cache: Any = field(default_factory=lambda: _new_probe_cache()) def spawn_for(self, provider: str | None, model: str | None, effort: str | None) -> Callable[..., Awaitable[Any]]: @@ -105,6 +107,12 @@ async def shutdown(self) -> None: self.sessions.clear() +def _new_probe_cache() -> Any: + from src.server.desktop_projects import ProbeCache + + return ProbeCache() + + def _token_ok(state: DesktopServeState, presented: str | None) -> bool: if not presented: return False diff --git a/src/server/desktop_sessions.py b/src/server/desktop_sessions.py index ab415e2e8..a0da35b40 100644 --- a/src/server/desktop_sessions.py +++ b/src/server/desktop_sessions.py @@ -14,6 +14,7 @@ import json import logging +import threading from pathlib import Path from typing import Any @@ -60,6 +61,22 @@ def _row_from_file(path: Path, data: dict[str, Any]) -> dict[str, Any]: } +# Sidebar rows by file path, keyed on the (mtime, size) they were read at. +# A session file is a whole conversation (up to a few MB), and the sidebar +# tree is rebuilt after every turn end and every session switch: re-parsing +# thousands of unchanged files each time cost ~1 s per rebuild. The stamp is +# re-checked with one ``stat`` per file, so an edited, replaced or deleted +# file is never served stale. +_ROW_CACHE: dict[str, tuple[tuple[int, int], dict[str, Any]]] = {} +_ROW_CACHE_LOCK = threading.Lock() + + +def clear_session_row_cache() -> None: + """Forget every cached sidebar row (tests, or a sessions-dir switch).""" + with _ROW_CACHE_LOCK: + _ROW_CACHE.clear() + + def list_session_rows( sessions_dir: Path, *, @@ -67,25 +84,47 @@ def list_session_rows( offset: int = 0, min_messages: int = 0, ) -> dict[str, Any]: - """Paginated sidebar listing, newest-first by file mtime.""" + """Paginated sidebar listing, newest-first by file mtime. + + Unchanged files come from :data:`_ROW_CACHE`; only a file whose + ``(mtime, size)`` moved since it was last read is parsed again. + """ + stamped: list[tuple[Path, tuple[int, int], float]] = [] try: - files = sorted( - sessions_dir.glob("*.json"), - key=lambda p: p.stat().st_mtime, - reverse=True, - ) + for path in sessions_dir.glob("*.json"): + try: + stat = path.stat() + except OSError: + continue # deleted between the listing and the stat + stamped.append((path, (stat.st_mtime_ns, stat.st_size), stat.st_mtime)) except OSError: - files = [] + stamped = [] + stamped.sort(key=lambda item: item[2], reverse=True) rows: list[dict[str, Any]] = [] - for path in files: - data = _read_session_file(path) - if data is None: - continue - row = _row_from_file(path, data) + seen: set[str] = set() + for path, stamp, _mtime in stamped: + key = str(path) + seen.add(key) + with _ROW_CACHE_LOCK: + cached = _ROW_CACHE.get(key) + if cached is not None and cached[0] == stamp: + row = cached[1] + else: + data = _read_session_file(path) + if data is None: + continue + row = _row_from_file(path, data) + with _ROW_CACHE_LOCK: + _ROW_CACHE[key] = (stamp, row) if row["message_count"] < min_messages: continue - rows.append(row) + # A copy per call: callers annotate rows (``is_active`` …) and the + # cached one must stay as the file said. + rows.append(dict(row)) + with _ROW_CACHE_LOCK: + for key in [k for k in _ROW_CACHE if k not in seen]: + _ROW_CACHE.pop(key, None) window = rows[offset : offset + limit] if limit > 0 else rows[offset:] return { @@ -224,12 +263,21 @@ def load_session_messages(sessions_dir: Path, session_id: str) -> dict[str, Any] # the sidebar reads it from this same file, and a header that disagreed # with the row the user just clicked is its own small confusion. name = data.get("name") - return { + result: dict[str, Any] = { "messages": messages, "message_count": len(messages), "session_id": str(data.get("session_id") or safe), "title": str(name) if isinstance(name, str) and name.strip() else "", } + # The facts a client needs to SHOW a stored session before any runtime + # exists for it: where it ran and what it ran on. Same keys as the + # ``session.info`` payload, so the header and the model chip read a cold + # transcript and a live one alike. + for key in ("cwd", "model", "provider"): + value = data.get(key) + if isinstance(value, str) and value: + result[key] = value + return result def _write_session_file(path: Path, data: dict[str, Any]) -> bool: diff --git a/tests/server/test_desktop_gateway.py b/tests/server/test_desktop_gateway.py index 171a77253..38578a5de 100644 --- a/tests/server/test_desktop_gateway.py +++ b/tests/server/test_desktop_gateway.py @@ -222,12 +222,14 @@ async def shutdown(self) -> None: class FakeManager: def __init__(self) -> None: self.created: list[str] = [] + self.cwds: list[str] = [] self._n = 0 def create_session(self, cwd: str): self._n += 1 session_id = f"fake-{self._n}" self.created.append(session_id) + self.cwds.append(cwd) return SimpleNamespace(id=session_id, cwd=cwd) def mark_running(self, session_id: str) -> None: @@ -1047,3 +1049,197 @@ def test_effort_change_round_trips_and_reports_persisted(tmp_path: Path) -> None _rpc(ws, 5, "config.set", {"session_id": sid, "key": "effort", "value": "bogus"}) result = _drain_for_response(ws, 5, events)["result"] assert result["ok"] is False and "invalid effort" in result["error"] + + +# ─── opening a saved session: cold history, runtime reuse ──────────────────── + + +def _write_saved(sessions_dir: Path, session_id: str, messages: list, **extra) -> None: + sessions_dir.mkdir(exist_ok=True) + payload = { + "session_id": session_id, + "preview": "hello?", + "message_count": len(messages), + "cwd": "/tmp/where", + "model": "m-stored", + "provider": "p-stored", + "conversation": {"messages": messages}, + **extra, + } + (sessions_dir / f"{session_id}.json").write_text(json.dumps(payload), encoding="utf-8") + + +def test_session_history_reads_the_saved_transcript_without_a_runtime(tmp_path: Path) -> None: + """The sidebar click renders from this — no spawn, no control round-trip.""" + state, agents = _fake_state(tmp_path) + state.sessions_dir = tmp_path / "saved" + _write_saved( + state.sessions_dir, "old-chat", + [{"role": "user", "content": "hello?"}, + {"role": "assistant", "content": [{"type": "text", "text": "hi back"}]}], + name="My chat", + ) + + with TestClient(build_app(state)) as client, _connect(client) as ws: + ws.receive_json() + events: list[dict] = [] + _rpc(ws, 1, "session.history", {"session_id": "old-chat"}) + result = _drain_for_response(ws, 1, events)["result"] + + assert result["found"] is True + assert result["stored_session_id"] == "old-chat" + assert result["title"] == "My chat" + assert [m["role"] for m in result["messages"]] == ["user", "assistant"] + assert result["info"] == {"cwd": "/tmp/where", "model": "m-stored", "provider": "p-stored"} + assert agents == [] and state.manager.created == [] + + +def test_session_history_of_an_unknown_row_is_an_error(tmp_path: Path) -> None: + state, _agents = _fake_state(tmp_path) + state.sessions_dir = tmp_path / "saved" + state.sessions_dir.mkdir() + + with TestClient(build_app(state)) as client, _connect(client) as ws: + ws.receive_json() + _rpc(ws, 1, "session.history", {"session_id": "nope"}) + reply = _drain_for_response(ws, 1, []) + + assert "unknown session" in reply["error"]["message"] + + +def test_resuming_the_same_row_twice_reuses_its_runtime(tmp_path: Path) -> None: + """Every click used to spawn a fresh runtime and leave the last one alive. + + ``state.sessions`` is keyed by runtime id, and a resumed row's runtime has + a different id from the row, so the "already live" check never matched a + row. The second resume must come back with the same runtime, spawn-free. + """ + state, agents = _fake_state(tmp_path) + state.sessions_dir = tmp_path / "saved" + _write_saved(state.sessions_dir, "old-chat", [{"role": "user", "content": "hello?"}]) + + with TestClient(build_app(state)) as client, _connect(client) as ws: + ws.receive_json() + events: list[dict] = [] + _rpc(ws, 1, "session.resume", {"session_id": "old-chat"}) + first = _drain_for_response(ws, 1, events)["result"] + _rpc(ws, 2, "session.resume", {"session_id": "old-chat", "omit_messages": True}) + second = _drain_for_response(ws, 2, events)["result"] + + assert first["session_id"] == "fake-1" + assert second["session_id"] == "fake-1" + assert second["stored_session_id"] == "old-chat" + assert second.get("messages_omitted") is True + assert len(agents) == 1 and state.manager.created == ["fake-1"] + assert state.sessions["fake-1"].stored_id == "old-chat" + + +def test_a_live_replay_answers_history_from_its_own_record(tmp_path: Path) -> None: + """Turns run after a resume are saved under the RUNTIME's id; the row the + user clicks still names the original file. Opening the row again must + show the conversation as it is now, not as the row's file left it.""" + state, _agents = _fake_state(tmp_path) + state.sessions_dir = tmp_path / "saved" + _write_saved(state.sessions_dir, "old-chat", [{"role": "user", "content": "first"}]) + + with TestClient(build_app(state)) as client, _connect(client) as ws: + ws.receive_json() + events: list[dict] = [] + _rpc(ws, 1, "session.resume", {"session_id": "old-chat", "omit_messages": True}) + runtime = _drain_for_response(ws, 1, events)["result"]["session_id"] + # The runtime saved its own, longer record after a turn. + _write_saved( + state.sessions_dir, runtime, + [{"role": "user", "content": "first"}, {"role": "user", "content": "second"}], + ) + _rpc(ws, 2, "session.history", {"session_id": "old-chat"}) + history = _drain_for_response(ws, 2, events)["result"] + _rpc(ws, 3, "session.resume", {"session_id": "old-chat"}) + resumed = _drain_for_response(ws, 3, events)["result"] + + assert history["live_session_id"] == runtime + assert [m["content"] for m in history["messages"]] == ["first", "second"] + assert [m["content"] for m in resumed["messages"]] == ["first", "second"] + + +def test_two_concurrent_resumes_of_one_row_share_a_spawn(tmp_path: Path) -> None: + state, agents = _fake_state(tmp_path) + state.sessions_dir = tmp_path / "saved" + _write_saved(state.sessions_dir, "old-chat", [{"role": "user", "content": "hello?"}]) + gate = asyncio.Event() + base_spawn = state.spawn_agent + + async def slow_spawn(session_id, cwd, resume): + await gate.wait() + return await base_spawn(session_id, cwd, resume) + + state.spawn_agent = slow_spawn + + async def run() -> tuple[dict, dict]: + from src.server.desktop_gateway_methods import GatewayConnection + + class _Socket: + async def send_json(self, obj): # pragma: no cover - no pushes read + pass + + conn = GatewayConnection(websocket=_Socket(), state=state) # type: ignore[arg-type] + first = asyncio.create_task(conn.session_resume({"session_id": "old-chat"})) + await asyncio.sleep(0) + second = asyncio.create_task(conn.session_resume({"session_id": "old-chat"})) + await asyncio.sleep(0) + gate.set() + return await first, await second + + a, b = asyncio.run(run()) + assert a["session_id"] == b["session_id"] == "fake-1" + assert len(agents) == 1 + + +# ─── session.create: a new folder, a worktree ──────────────────────────────── + + +def test_session_create_can_make_the_workspace_folder(tmp_path: Path) -> None: + state, _agents = _fake_state(tmp_path) + target = tmp_path / "fresh" / "project" + + with TestClient(build_app(state)) as client, _connect(client) as ws: + ws.receive_json() + _rpc(ws, 1, "session.create", {"cwd": str(target)}) + refused = _drain_for_response(ws, 1, []) + _rpc(ws, 2, "session.create", {"cwd": str(target), "create_dir": True}) + created = _drain_for_response(ws, 2, [])["result"] + _rpc(ws, 3, "session.create", {"cwd": "relative/path", "create_dir": True}) + relative = _drain_for_response(ws, 3, []) + + assert "no such directory" in refused["error"]["message"] + assert target.is_dir() + assert created["session_id"] == "fake-1" + assert state.manager.cwds == [str(target)] + assert "must be absolute" in relative["error"]["message"] + + +def test_session_create_can_isolate_the_session_in_a_worktree(tmp_path: Path) -> None: + import subprocess + + repo = tmp_path / "repo" + repo.mkdir() + env = {"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@t", "PATH": __import__("os").environ["PATH"], + "HOME": str(tmp_path)} + for args in (["init", "-q", "-b", "main"], ["commit", "-q", "--allow-empty", "-m", "root"]): + subprocess.run(["git", *args], cwd=repo, check=True, env=env) + state, _agents = _fake_state(tmp_path) + + with TestClient(build_app(state)) as client, _connect(client) as ws: + ws.receive_json() + _rpc(ws, 1, "session.create", {"cwd": str(repo), "worktree": True}) + created = _drain_for_response(ws, 1, [])["result"] + _rpc(ws, 2, "session.create", {"cwd": str(tmp_path), "worktree": True}) + refused = _drain_for_response(ws, 2, []) + + worktree = created["worktree"] + assert Path(worktree["path"]).is_dir() + assert Path(worktree["path"]).parent == repo / ".clawcodex" / "worktrees" + assert worktree["repo_root"] == str(repo.resolve()) + assert state.manager.cwds == [worktree["path"]] + assert "git repository" in refused["error"]["message"] diff --git a/tests/server/test_desktop_projects.py b/tests/server/test_desktop_projects.py index 142c0d64f..4182aac48 100644 --- a/tests/server/test_desktop_projects.py +++ b/tests/server/test_desktop_projects.py @@ -183,3 +183,39 @@ def test_unresolved_cwd_falls_into_home_bucket(tmp_path): ) assert [project["id"] for project in tree["projects"]] == [NO_PROJECT_ID] + + +# ─── ProbeCache ────────────────────────────────────────────────────────────── + + +def test_probe_cache_answers_within_its_ttl_and_forgets_after(): + from src.server.desktop_projects import ProbeCache + + now = [100.0] + cache = ProbeCache(ttl_s=10.0, worktree_ttl_s=2.0, clock=lambda: now[0]) + + assert cache.has_repo_root("/a") is False + cache.set_repo_root("/a", "/repo") + cache.set_repo_root("/b", None) # "not a repo" is an answer too + cache.set_worktrees("/repo", ["/repo", "/repo/.wt/x"]) + + assert cache.has_repo_root("/a") and cache.repo_root("/a") == "/repo" + assert cache.has_repo_root("/b") and cache.repo_root("/b") is None + assert cache.worktrees("/repo") == ["/repo", "/repo/.wt/x"] + + now[0] += 3.0 + assert cache.worktrees("/repo") is None # worktree lists expire sooner + assert cache.has_repo_root("/a") + now[0] += 8.0 + assert cache.has_repo_root("/a") is False + + +def test_probe_cache_forget_worktrees_drops_one_repo(): + from src.server.desktop_projects import ProbeCache + + cache = ProbeCache() + cache.set_worktrees("/r1", ["/r1"]) + cache.set_worktrees("/r2", ["/r2"]) + cache.forget_worktrees("/r1") + assert cache.worktrees("/r1") is None + assert cache.worktrees("/r2") == ["/r2"] diff --git a/tests/server/test_desktop_sessions.py b/tests/server/test_desktop_sessions.py index 416d2df6f..92f758296 100644 --- a/tests/server/test_desktop_sessions.py +++ b/tests/server/test_desktop_sessions.py @@ -522,3 +522,61 @@ def test_conversation_keeps_a_step_usage_and_model_on_disk() -> None: assert stored[1]["usage"] == {"input_tokens": 3, "output_tokens": 2} assert stored[1]["model"] == "deepseek-v4-flash" assert "usage" not in stored[0] + + +# ─── the sidebar row cache ─────────────────────────────────────────────────── + + +def test_list_session_rows_rereads_only_changed_files(tmp_path: Path, monkeypatch) -> None: + from src.server import desktop_sessions + + desktop_sessions.clear_session_row_cache() + d = tmp_path / "sessions" + d.mkdir() + _write_session(d, "a", preview="A", count=1, age_s=20) + _write_session(d, "b", preview="B", count=1, age_s=10) + reads: list[str] = [] + real = desktop_sessions._read_session_file + + def counting(path): + reads.append(path.stem) + return real(path) + + monkeypatch.setattr(desktop_sessions, "_read_session_file", counting) + + first = desktop_sessions.list_session_rows(d, limit=0) + assert [r["id"] for r in first["sessions"]] == ["b", "a"] + assert sorted(reads) == ["a", "b"] + + # Nothing changed: no file is parsed again, rows are equal but not shared. + second = desktop_sessions.list_session_rows(d, limit=0) + assert sorted(reads) == ["a", "b"] + assert second["sessions"] == first["sessions"] + second["sessions"][0]["is_active"] = True + assert desktop_sessions.list_session_rows(d, limit=0)["sessions"][0]["is_active"] is False + + # A rewritten file is parsed again and its new content served. + _write_session(d, "a", preview="A2", count=3) + third = desktop_sessions.list_session_rows(d, limit=0) + assert reads.count("a") == 2 + assert [r["id"] for r in third["sessions"]] == ["a", "b"] + assert third["sessions"][0]["preview"] == "A2" + + # A deleted file leaves the cache too. + (d / "b.json").unlink() + assert [r["id"] for r in desktop_sessions.list_session_rows(d, limit=0)["sessions"]] == ["a"] + assert "b.json" not in " ".join(desktop_sessions._ROW_CACHE) + desktop_sessions.clear_session_row_cache() + + +def test_load_session_messages_carries_the_stored_session_facts(tmp_path: Path) -> None: + from src.server.desktop_sessions import load_session_messages + + d = tmp_path / "sessions" + d.mkdir() + _write_session(d, "s", preview="P", count=1, messages=[{"role": "user", "content": "hi"}]) + + stored = load_session_messages(d, "s") + + assert stored is not None + assert (stored["cwd"], stored["model"], stored["provider"]) == ("/tmp/w", "m1", "p1") diff --git a/tests/test_workspace_snapshot.py b/tests/test_workspace_snapshot.py new file mode 100644 index 000000000..f00da5d8b --- /dev/null +++ b/tests/test_workspace_snapshot.py @@ -0,0 +1,118 @@ +"""The workspace snapshot's file walk: pruned before entering, bounded, honest. + +The system prompt's ``## Runtime Context`` counts used to come from two +``Path.rglob`` passes that crawled every directory under the workspace — +``node_modules`` included — and filtered afterwards, ~20 s per system-prompt +build on a repo with a few front-end packages (built at spawn and again on +every resume, so ~45 s to open a saved session from the web sidebar). +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from src.context_system.builder import _build_workspace_section +from src.context_system.workspace_snapshot import ( + MAX_SCANNED_DIRS, + build_workspace_snapshot, + count_python_files, +) + + +def _touch(path: Path) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("", encoding="utf-8") + + +def test_counts_python_and_test_files_outside_ignored_trees(tmp_path: Path) -> None: + _touch(tmp_path / "src" / "app.py") + _touch(tmp_path / "src" / "pkg" / "util.py") + _touch(tmp_path / "tests" / "test_app.py") + _touch(tmp_path / "README.md") + # Vendored and generated trees must neither count nor be entered. + _touch(tmp_path / "node_modules" / "left-pad" / "setup.py") + _touch(tmp_path / ".venv" / "lib" / "site.py") + _touch(tmp_path / ".git" / "hooks" / "test_hook.py") + _touch(tmp_path / ".clawcodex" / "worktrees" / "wt-1" / "src" / "app.py") + + snapshot = build_workspace_snapshot(tmp_path) + + assert snapshot.python_file_count == 3 + assert snapshot.test_file_count == 1 + assert snapshot.counts_partial is False + assert snapshot.key_files == ("README.md",) + assert "node_modules/" not in snapshot.top_level_entries + assert "src/" in snapshot.top_level_entries + + +def test_ignored_directories_are_pruned_not_filtered(tmp_path: Path) -> None: + """A huge ignored subtree costs nothing: the walk never steps into it. + + With a budget of three directories (root, ``src``, ``tests``) the walk + only stays within budget if ``node_modules`` — deeper than the budget on + its own — was skipped before being entered rather than crawled and then + filtered out. + """ + _touch(tmp_path / "src" / "app.py") + _touch(tmp_path / "tests" / "test_app.py") + deep = tmp_path / "node_modules" + for index in range(20): + deep = deep / f"dep-{index}" + _touch(deep / "vendored.py") + + python_files, test_files, partial = count_python_files(tmp_path, max_dirs=3) + + assert (python_files, test_files, partial) == (2, 1, False) + + +def test_the_walk_stops_at_its_budget_and_says_so(tmp_path: Path) -> None: + for index in range(10): + _touch(tmp_path / f"pkg-{index:02d}" / "mod.py") + + python_files, _tests, partial = count_python_files(tmp_path, max_dirs=4) + + # Root plus the first three packages in name order — deterministic. + assert partial is True + assert python_files == 3 + + snapshot = build_workspace_snapshot(tmp_path, max_dirs=4) + assert snapshot.counts_partial is True + + section = _build_workspace_section(tmp_path, tmp_path) + assert "- Python files: 10" in section # the default budget covers it all + assert "partial" not in section + + +def test_partial_counts_are_rendered_as_lower_bounds(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + for index in range(6): + _touch(tmp_path / f"pkg-{index}" / "test_mod.py") + + from src.context_system import workspace_snapshot + + monkeypatch.setattr(workspace_snapshot, "MAX_SCANNED_DIRS", 2) + # The builder calls through the module-level default, so patch the + # function's default the way a smaller budget would arrive in production. + monkeypatch.setattr( + workspace_snapshot, + "build_workspace_snapshot", + lambda root, cwd=None: build_workspace_snapshot(root, cwd=cwd, max_dirs=2), + ) + + section = _build_workspace_section(tmp_path, tmp_path) + + assert "- Python files: 1+ (partial scan)" in section + assert "- Test files: 1+ (partial scan)" in section + + +def test_symlink_loops_do_not_hang_the_walk(tmp_path: Path) -> None: + _touch(tmp_path / "src" / "app.py") + try: + (tmp_path / "src" / "loop").symlink_to(tmp_path, target_is_directory=True) + except (OSError, NotImplementedError): # pragma: no cover - platform + pytest.skip("symlinks unavailable") + + python_files, _tests, partial = count_python_files(tmp_path, max_dirs=MAX_SCANNED_DIRS) + + assert (python_files, partial) == (1, False) diff --git a/ui-web/README.md b/ui-web/README.md index f283dc4a6..6b91a4c31 100644 --- a/ui-web/README.md +++ b/ui-web/README.md @@ -237,6 +237,46 @@ Harness, selecting a session opens its folder unless it was manually collapsed; manual toggles last for the page's lifetime. Filtering temporarily expands matching folders and restores their previous state when cleared. +## Opening a saved session + +Clicking a row is two round-trips, the way the reference opens a session. +`session.history` reads the stored transcript cold — a file read, tens of +milliseconds even for a multi-megabyte conversation — and the client renders +it at once: title, workspace, model chip, nodes, trajectory timings. Then +`session.resume` attaches the runtime that will answer the next prompt +(provider, tool registry, system prompt, the stored conversation loaded into +it) behind the transcript; the composer says *Connecting the agent…* +meanwhile, and a prompt sent during that window waits for the attach rather +than starting a session of its own. Before this the whole click sat on the +attach, and the attach sat on a system-prompt walk of the workspace that +took ~20 s per build on a repo with a few `node_modules` trees (built at +spawn and again on resume — ~45 s to open a session). + +A row the backend has already replayed comes back with the same runtime: +the backend keys live sessions by runtime id, matches a resume on the stored +id the runtime replays, and a second click while the first is still +attaching waits on it instead of spawning twice. Leaving a session releases +its runtime (`session.close`) when nothing is happening in it — no turn +running, no approval or question pending, no prompts queued — so browsing +through saved sessions does not leave a trail of idle agents; the +conversation itself is saved under the runtime's id at every turn end and +replays from there. A backend without `session.history` gets the one-call +resume, transcript included, as before. + +## New session + +**New session** (the sidebar button, the brand mark, `⌘⇧N`) opens a dialog +rather than starting a session on the spot. It offers every workspace the +sidebar knows plus **Create new workspace…**, which takes an absolute folder +path and creates the folder if it is not there yet (`session.create` with +`create_dir`). The **Worktree** switch runs the session in a fresh git +worktree of that repo — the CLI's `--worktree`, under +`.clawcodex/worktrees/` — so parallel sessions in one repo cannot step +on each other's files; the worktree is left in place when the session ends, +since a browser tab has no exit dialog to offer keep-or-remove. A refusal +(a relative path, a folder that is not a git repository) stays in the dialog +for correcting. + ## The session across a reload A reload lands back on the session the window was on. The client remembers @@ -248,9 +288,10 @@ the backend still has it, the reply is the very same session, its running turn included; once it is gone, the runtime's own record — the complete one — is replayed into a new runtime. A runtime that never saved, because nothing was typed after resuming a row, has no record, so the row it came from is -replayed instead and the blank runtime the first attempt spawned is closed. -A session the backend no longer knows is forgotten without a notice: the -hero is the honest place to land. +replayed instead (which releases the blank runtime the first attempt landed +on, as any navigation away from an idle runtime does). A session the backend +no longer knows is forgotten without a notice: the hero is the honest place +to land. ## Trajectory diff --git a/ui-web/src/App.tsx b/ui-web/src/App.tsx index 07dbd214e..790614106 100644 --- a/ui-web/src/App.tsx +++ b/ui-web/src/App.tsx @@ -6,8 +6,9 @@ import { SettingsOverlay } from './settings/SettingsOverlay.tsx' import { SidebarRight } from './sidebar-right/SidebarRight.tsx' import { closeSidebar, resetSidebar } from './sidebar-right/store.ts' import { AppFrame } from './layout/AppFrame.tsx' +import { NewSessionDialog, openNewSessionDialog } from './sidebar/NewSessionDialog.tsx' import { Sidebar } from './sidebar/Sidebar.tsx' -import { createSession, start } from './state/actions.ts' +import { start } from './state/actions.ts' import { $detailsOpen, $detailsWidth, openDetails, toggleSidebar } from './state/layout.ts' import { $bootError, $bootPhase, $sessionId, $workspace } from './state/store.ts' import { installTheme } from './state/theme.ts' @@ -97,7 +98,7 @@ export function App() { // stealing it would surprise the user in their own browser. if (event.key === 'n' && event.shiftKey) { event.preventDefault() - void createSession({ cwd: $workspace.get() }) + openNewSessionDialog() } } @@ -121,9 +122,10 @@ export function App() { details={detailsOpen ? : null} sidebar={state => } /> - {/* Outside the frame: it covers the whole app, including the sidebar - it is opened from. */} + {/* Outside the frame: they cover the whole app, including the sidebar + they are opened from. */} + ) } diff --git a/ui-web/src/conversation/InputBar.tsx b/ui-web/src/conversation/InputBar.tsx index db3bf9686..28277e23e 100644 --- a/ui-web/src/conversation/InputBar.tsx +++ b/ui-web/src/conversation/InputBar.tsx @@ -17,7 +17,7 @@ import type { ModelOptionsResult, } from '../gateway/protocol.ts' import { attachImage, searchFiles } from '../state/actions.ts' -import { $commands, $notice } from '../state/store.ts' +import { $commands, $notice, $sessionAttaching } from '../state/store.ts' import { ArrowUpIcon, PlusIcon, SlashSquareIcon, StopIcon, XIcon } from '../ui/icons.tsx' import { ContextMeter } from './ContextMeter.tsx' import { @@ -112,6 +112,7 @@ export function InputBar({ }: InputBarProps) { const commands = useStore($commands) const notice = useStore($notice) + const attaching = useStore($sessionAttaching) const textarea = useRef(null) const card = useRef(null) const [highlight, setHighlight] = useState(0) @@ -484,7 +485,7 @@ export function InputBar({ return (
- {notice.text !== '' && ( + {notice.text !== '' ? (
{notice.text}
+ ) : ( + // The transcript is up before its runtime is: say so, since a prompt + // sent now waits for the agent rather than going out at once. + attaching && ( +
+ Connecting the agent… +
+ ) )}
{mention !== null && files.length > 0 && ( diff --git a/ui-web/src/gateway/protocol.ts b/ui-web/src/gateway/protocol.ts index 1c6d46891..b061e4de8 100644 --- a/ui-web/src/gateway/protocol.ts +++ b/ui-web/src/gateway/protocol.ts @@ -243,6 +243,36 @@ export interface SessionCreateResult { info?: SessionInfoPayload session_id: string stored_session_id?: string + /** Present when the session was created with `worktree: true`. */ + worktree?: SessionWorktree +} + +/** The git worktree a session was isolated in (`session.create` → `worktree`). */ +export interface SessionWorktree { + branch?: string + name?: string + path: string + repo_root?: string +} + +/** + * `session.history` — a saved session's transcript, read cold: no runtime is + * spawned or asked. What the sidebar click renders at once; the runtime + * attaches afterwards through `session.resume`. + * + * `found` is false for a live runtime that never saved (nothing typed into it + * yet): no messages, and not an error. `live_session_id` names the runtime + * already replaying this row when the backend has one. + */ +export interface SessionHistoryResult { + found?: boolean + info?: SessionInfoPayload + live_session_id?: string + message_count?: number + messages?: StoredMessage[] + session_id?: string + stored_session_id?: string + title?: string } export interface StoredMessage { diff --git a/ui-web/src/sidebar/NewSessionDialog.module.css b/ui-web/src/sidebar/NewSessionDialog.module.css new file mode 100644 index 000000000..60de25eaf --- /dev/null +++ b/ui-web/src/sidebar/NewSessionDialog.module.css @@ -0,0 +1,171 @@ +/* The New session dialog. Same chrome as the Full-access confirmation: a + centred card on the mask, above the composer's menus and the settings + takeover. */ + +.scrim { + position: fixed; + inset: 0; + z-index: 300; + display: grid; + place-items: center; + padding: 24px; + background: var(--cc-alias-bg-mask-1); +} + +.dialog { + box-sizing: border-box; + display: flex; + flex-direction: column; + gap: 14px; + width: min(440px, 100%); + margin: 0; + padding: 18px; + border: 1px solid var(--cc-alias-border-inverted); + border-radius: 14px; + background: var(--cc-specific-menu); + box-shadow: var(--cc-shadow-lv3); +} + +.head { + display: flex; + align-items: center; + justify-content: space-between; +} + +.title { + color: var(--cc-alias-label-primary); + font-size: 15px; + font-weight: 600; + line-height: 22px; +} + +.close { + display: inline-flex; + align-items: center; + justify-content: center; + width: 24px; + height: 24px; + padding: 0; + border: none; + border-radius: 50%; + background: transparent; + color: var(--cc-alias-label-secondary); + cursor: pointer; +} + +.close:hover { + background: var(--cc-alias-interactive-bg-hover); +} + +.field { + display: flex; + flex-direction: column; + gap: 6px; +} + +.label { + color: var(--cc-alias-label-primary); + font-size: 13px; + font-weight: 500; + line-height: 18px; +} + +.hint { + color: var(--cc-alias-label-tertiary); + font-size: 12px; + line-height: 16px; +} + +.select, +.input { + box-sizing: border-box; + width: 100%; + height: 34px; + padding: 0 10px; + border: 1px solid var(--cc-alias-border-l3); + border-radius: 8px; + background: var(--cc-alias-bg-layer-1); + color: var(--cc-alias-label-primary); + font: inherit; + font-size: 13px; +} + +.select:focus-visible, +.input:focus-visible { + outline: 2px solid var(--cc-alias-button-info-fill); + outline-offset: -1px; +} + +.input::placeholder { + color: var(--cc-alias-label-caption); +} + +.switchRow { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + cursor: pointer; +} + +.switchText { + display: flex; + flex-direction: column; + gap: 2px; +} + +/* A switch drawn from the checkbox itself, so the state stays in the form. */ +.switch { + appearance: none; + flex: none; + position: relative; + width: 36px; + height: 20px; + margin: 0; + border-radius: 10px; + background: var(--cc-alias-border-l4); + cursor: pointer; + transition: background 120ms ease; +} + +.switch::after { + content: ''; + position: absolute; + top: 2px; + left: 2px; + width: 16px; + height: 16px; + border-radius: 50%; + background: var(--cc-static-neutral-00); + box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2); + transition: transform 120ms ease; +} + +.switch:checked { + background: var(--cc-alias-button-info-fill); +} + +.switch:checked::after { + transform: translateX(16px); +} + +.switch:focus-visible { + outline: 2px solid var(--cc-alias-button-info-fill); + outline-offset: 2px; +} + +.error { + padding: 8px 10px; + border-radius: 8px; + background: var(--cc-alias-interactive-bg-hover-danger); + color: var(--cc-static-red-600); + font-size: 12px; + line-height: 18px; + overflow-wrap: anywhere; +} + +.actions { + display: flex; + justify-content: flex-end; + gap: 8px; +} diff --git a/ui-web/src/sidebar/NewSessionDialog.test.tsx b/ui-web/src/sidebar/NewSessionDialog.test.tsx new file mode 100644 index 000000000..673d02659 --- /dev/null +++ b/ui-web/src/sidebar/NewSessionDialog.test.tsx @@ -0,0 +1,129 @@ +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { ProjectNode } from '../gateway/protocol.ts' +import { $newSessionDialog, $projects, $workspace } from '../state/store.ts' + +const createSession = vi.fn<(options?: Record) => Promise>() + +vi.mock('../state/actions.ts', () => ({ + createSession: (options?: Record) => createSession(options), +})) + +const { NEW_WORKSPACE, NewSessionDialog, knownWorkspaces, openNewSessionDialog } = await import('./NewSessionDialog.tsx') + +function project(path: string | null): ProjectNode { + return { id: path ?? 'home', label: path ?? 'Home', path, repos: [] } +} + +beforeEach(() => { + createSession.mockReset() + createSession.mockResolvedValue(null) + $workspace.set('/work/current') + $projects.set([project('/work/alpha'), project('/work/current'), project(null)]) + $newSessionDialog.set(false) +}) + +afterEach(() => { + cleanup() + $newSessionDialog.set(false) + $projects.set([]) + $workspace.set('') +}) + +describe('knownWorkspaces', () => { + it('lists the current workspace first, then the sidebar folders, once each, never Home', () => { + expect(knownWorkspaces('/work/current', [project('/work/alpha'), project('/work/current'), project(null)])) + .toEqual(['/work/current', '/work/alpha']) + expect(knownWorkspaces('', [project('/a')])).toEqual(['/a']) + }) +}) + +describe('NewSessionDialog', () => { + it('is closed until opened, and starts a session in the chosen workspace', async () => { + render() + + expect(screen.queryByRole('dialog')).toBeNull() + + act(() => { + openNewSessionDialog() + }) + + const select = screen.getByRole('combobox') as HTMLSelectElement + expect(select.value).toBe('/work/current') + expect(screen.queryByPlaceholderText('/absolute/path/to/project')).toBeNull() + + fireEvent.change(select, { target: { value: '/work/alpha' } }) + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + await act(async () => { + await Promise.resolve() + }) + + expect(createSession).toHaveBeenCalledWith({ cwd: '/work/alpha' }) + expect($newSessionDialog.get()).toBe(false) + }) + + it('creates a new workspace from an absolute path, in a worktree when asked', async () => { + $newSessionDialog.set(true) + render() + + const select = screen.getByRole('combobox') as HTMLSelectElement + fireEvent.change(select, { target: { value: NEW_WORKSPACE } }) + + const create = screen.getByRole('button', { name: 'Create' }) as HTMLButtonElement + expect(create.disabled).toBe(true) + + fireEvent.change(screen.getByPlaceholderText('/absolute/path/to/project'), { + target: { value: '/work/fresh ' }, + }) + fireEvent.click(screen.getByRole('switch')) + expect(create.disabled).toBe(false) + + fireEvent.click(create) + await act(async () => { + await Promise.resolve() + }) + + expect(createSession).toHaveBeenCalledWith({ createDir: true, cwd: '/work/fresh', worktree: true }) + expect($newSessionDialog.get()).toBe(false) + }) + + it('keeps the dialog open with the reason when the backend refuses', async () => { + createSession.mockResolvedValue('no such directory: /nope') + $newSessionDialog.set(true) + render() + + fireEvent.click(screen.getByRole('button', { name: 'Create' })) + await act(async () => { + await Promise.resolve() + }) + + expect(screen.getByRole('alert').textContent).toBe('no such directory: /nope') + expect($newSessionDialog.get()).toBe(true) + }) + + it('closes on Cancel and on Escape', () => { + $newSessionDialog.set(true) + render() + + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect($newSessionDialog.get()).toBe(false) + + act(() => { + openNewSessionDialog() + }) + fireEvent.keyDown(document, { key: 'Escape' }) + expect($newSessionDialog.get()).toBe(false) + expect(createSession).not.toHaveBeenCalled() + }) + + it('offers only the new-workspace path when no workspace is known', () => { + $workspace.set('') + $projects.set([]) + $newSessionDialog.set(true) + render() + + expect((screen.getByRole('combobox') as HTMLSelectElement).value).toBe(NEW_WORKSPACE) + expect(screen.getByPlaceholderText('/absolute/path/to/project')).toBeTruthy() + }) +}) diff --git a/ui-web/src/sidebar/NewSessionDialog.tsx b/ui-web/src/sidebar/NewSessionDialog.tsx new file mode 100644 index 000000000..73a9e297e --- /dev/null +++ b/ui-web/src/sidebar/NewSessionDialog.tsx @@ -0,0 +1,213 @@ +import { useStore } from '@nanostores/react' +import { useEffect, useMemo, useState, type FormEvent } from 'react' + +import { createSession } from '../state/actions.ts' +import { $newSessionDialog, $projects, $workspace } from '../state/store.ts' +import { Button } from '../ui/primitives/Button.tsx' +import { XIcon } from '../ui/icons.tsx' +import css from './NewSessionDialog.module.css' + +/** The select value that reveals the folder-path field. */ +export const NEW_WORKSPACE = '__new_workspace__' + +/** The last path segment, for a label; the whole path when it has none. */ +function baseName(path: string): string { + const segments = path.split(/[/\\]/).filter(Boolean) + + return segments[segments.length - 1] ?? path +} + +/** + * The workspaces the dialog offers: the current one first, then every folder + * the sidebar knows a session in, without repeats. "Home" (sessions with no + * folder) has no path to start a session in, so it is not a choice. + */ +export function knownWorkspaces(current: string, projects: readonly { path?: string | null }[]): string[] { + const seen = new Set() + const paths: string[] = [] + + for (const path of [current, ...projects.map(project => project.path ?? '')]) { + if (path === '' || seen.has(path)) continue + + seen.add(path) + paths.push(path) + } + + return paths +} + +export function openNewSessionDialog(): void { + $newSessionDialog.set(true) +} + +export function closeNewSessionDialog(): void { + $newSessionDialog.set(false) +} + +/** + * The New session dialog: which workspace, or a new one, and whether to + * isolate the session in a git worktree. + * + * A session runs somewhere, and until now the only somewhere was the current + * workspace: starting work in another project meant browsing to it first. + * The dialog puts the choice where the intent is. "Create new workspace…" + * takes an absolute path and makes the folder if it is not there yet; the + * worktree switch runs the session in a fresh checkout of the repo, the + * CLI's `--worktree`, so parallel sessions cannot step on each other's files. + * Errors stay in the dialog: a path the backend refuses is corrected here, + * not read off a status line behind a closed dialog. + */ +export function NewSessionDialog() { + const open = useStore($newSessionDialog) + + if (!open) return null + + return +} + +function NewSessionForm() { + const workspace = useStore($workspace) + const projects = useStore($projects) + const workspaces = useMemo(() => knownWorkspaces(workspace, projects), [projects, workspace]) + const [choice, setChoice] = useState(() => workspaces[0] ?? NEW_WORKSPACE) + const [path, setPath] = useState('') + const [worktree, setWorktree] = useState(false) + const [error, setError] = useState('') + const [creating, setCreating] = useState(false) + + const close = closeNewSessionDialog + + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + event.stopPropagation() + closeNewSessionDialog() + } + } + + document.addEventListener('keydown', onKeyDown) + + return () => { + document.removeEventListener('keydown', onKeyDown) + } + }, []) + + const creatingNew = choice === NEW_WORKSPACE + const target = creatingNew ? path.trim() : choice + const canCreate = target !== '' && !creating + + const onSubmit = async (event: FormEvent) => { + event.preventDefault() + + if (!canCreate) return + + setCreating(true) + setError('') + + const failure = await createSession({ + cwd: target, + ...(creatingNew && { createDir: true }), + ...(worktree && { worktree: true }), + }) + + setCreating(false) + + if (failure === null) close() + else setError(failure) + } + + return ( +
+
{ + event.stopPropagation() + }} + onSubmit={event => { + void onSubmit(event) + }} + role="dialog" + > +
+ + New session + + +
+ + + + {creatingNew && ( + + )} + + + + {error !== '' && ( +
+ {error} +
+ )} + +
+ + +
+
+
+ ) +} diff --git a/ui-web/src/sidebar/Sidebar.test.tsx b/ui-web/src/sidebar/Sidebar.test.tsx index e2afd1d92..303a6f1dd 100644 --- a/ui-web/src/sidebar/Sidebar.test.tsx +++ b/ui-web/src/sidebar/Sidebar.test.tsx @@ -144,3 +144,16 @@ describe('workspace folder expansion', () => { expect(screen.getByText('gamma conversation')).toBeTruthy() }) }) + +describe('New session', () => { + it('opens the dialog instead of starting a session on the spot', async () => { + const { $newSessionDialog } = await import('../state/store.ts') + $newSessionDialog.set(false) + + render() + fireEvent.click(screen.getByRole('button', { name: 'New session' })) + + expect($newSessionDialog.get()).toBe(true) + $newSessionDialog.set(false) + }) +}) diff --git a/ui-web/src/sidebar/Sidebar.tsx b/ui-web/src/sidebar/Sidebar.tsx index 6c37f3b27..d3a34f656 100644 --- a/ui-web/src/sidebar/Sidebar.tsx +++ b/ui-web/src/sidebar/Sidebar.tsx @@ -2,7 +2,7 @@ import { useStore } from '@nanostores/react' import { useEffect, useMemo, useState } from 'react' import type { ProjectNode, SessionRow } from '../gateway/protocol.ts' -import { createSession, resumeSession } from '../state/actions.ts' +import { resumeSession } from '../state/actions.ts' import { toggleSidebar } from '../state/layout.ts' import { BrandMark } from '../ui/BrandMark.tsx' import { @@ -28,6 +28,7 @@ import { XIcon, } from '../ui/icons.tsx' import { filterProjects, isBlankSession, visibleSessions } from './filter.ts' +import { openNewSessionDialog } from './NewSessionDialog.tsx' import { absoluteTime, relativeTime } from './recency.ts' import css from './Sidebar.module.css' @@ -218,9 +219,7 @@ export function Sidebar({ collapsed }: SidebarProps) { {!collapsed && (