From ec9bcdcdadf3450beb6e727807f23a6d1c2bbd16 Mon Sep 17 00:00:00 2001 From: serxa Date: Mon, 10 Aug 2026 20:53:23 +0000 Subject: [PATCH 1/2] Local code review in the session file-changes panel (code_review) Browser panel for reviewing on-disk git worktrees with line-anchored, two-way review comments before anything is committed or pushed. Adds the code_review config section, ReviewStore + migration, the gitreview gateway and /api/review routes, and the web review panel folded into the session file-changes view. Rebased onto current main: the code_review migration is numbered v045 (main took v040-v044), and CodeReviewConfig.from_dict routes through @_coerced like every other config section so string env values coerce to their declared types. Co-Authored-By: Claude Opus 4.8 --- nerve/config.py | 40 ++ nerve/db/base.py | 2 + nerve/db/migrations/v045_code_reviews.py | 70 ++++ nerve/db/reviews.py | 211 ++++++++++ nerve/gateway/gitreview.py | 164 ++++++++ nerve/gateway/routes/__init__.py | 2 + nerve/gateway/routes/reviews.py | 349 ++++++++++++++++ tests/test_reviews.py | 266 ++++++++++++ web/src/api/client.ts | 46 +++ web/src/components/Chat/FileChangesPanel.tsx | 404 +++++++++++++------ web/src/components/Review/ReviewDiff.tsx | 261 ++++++++++++ web/src/pages/ChatPage.tsx | 26 +- web/src/stores/reviewStore.ts | 248 ++++++++++++ web/src/types/review.ts | 60 +++ 14 files changed, 2013 insertions(+), 136 deletions(-) create mode 100644 nerve/db/migrations/v045_code_reviews.py create mode 100644 nerve/db/reviews.py create mode 100644 nerve/gateway/gitreview.py create mode 100644 nerve/gateway/routes/reviews.py create mode 100644 tests/test_reviews.py create mode 100644 web/src/components/Review/ReviewDiff.tsx create mode 100644 web/src/stores/reviewStore.ts create mode 100644 web/src/types/review.ts diff --git a/nerve/config.py b/nerve/config.py index 72bb3068..447b0644 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -2080,6 +2080,44 @@ def from_dict(cls, d: dict) -> McpEndpointConfig: ) +@dataclass +class CodeReviewConfig: + """Local code-review panel — browse on-disk git worktrees and exchange + line-anchored review comments with the agent, before anything is + committed or pushed. + + Off by default; set ``enabled: true`` and list the repository roots you + want reviewable under ``code_review`` in config.local.yaml, e.g.:: + + code_review: + enabled: true + repos: + - ~/nerve + - ~/project + + Only files inside a configured repo root (or one of its git worktrees) + are served. Authenticated with the existing web-UI JWT — same token + mechanism as the rest of the API. + """ + + enabled: bool = False + repos: list[str] = field(default_factory=list) + max_file_bytes: int = 2_000_000 # skip diffing/serving files larger than this + + @classmethod + @_coerced + def from_dict(cls, d: dict) -> "CodeReviewConfig": + # Pass raw values through; @_coerced normalizes them to the declared + # field types (str->bool for `enabled`, a bare scalar->one-element list + # for `repos`, str->int for `max_file_bytes`) the same way every other + # config section is coerced. Casting here would defeat that. + return cls( + enabled=d.get("enabled", False), + repos=d.get("repos") or [], + max_file_bytes=d.get("max_file_bytes", 2_000_000), + ) + + @dataclass class ExternalAgentTargetConfig: """One configured external agent (Codex, Claude Code, ...). @@ -2618,6 +2656,7 @@ class NerveConfig: mcp_endpoint: McpEndpointConfig = field(default_factory=McpEndpointConfig) mcp_servers: list[McpServerConfig] = field(default_factory=list) external_agents: ExternalAgentsConfig = field(default_factory=ExternalAgentsConfig) + code_review: CodeReviewConfig = field(default_factory=CodeReviewConfig) # API keys (from config.local.yaml) anthropic_api_key: str = "" @@ -2846,6 +2885,7 @@ def _build_from_dict(cls, d: dict) -> NerveConfig: mcp_endpoint=McpEndpointConfig.from_dict(d.get("mcp_endpoint", {})), mcp_servers=_parse_mcp_servers(d), external_agents=ExternalAgentsConfig.from_dict(d.get("external_agents", {})), + code_review=CodeReviewConfig.from_dict(d.get("code_review", {})), anthropic_api_key=d.get("anthropic_api_key", ""), openai_api_key=d.get("openai_api_key", ""), brave_search_api_key=d.get("brave_search_api_key", ""), diff --git a/nerve/db/base.py b/nerve/db/base.py index 0fc4c531..c15d0e37 100644 --- a/nerve/db/base.py +++ b/nerve/db/base.py @@ -25,6 +25,7 @@ from nerve.db.notifications import NotificationStore from nerve.db.plans import PlanStore from nerve.db.review_loops import ReviewLoopStore +from nerve.db.reviews import ReviewStore from nerve.db.sessions import SessionStore from nerve.db.skills import SkillStore from nerve.db.sources import SourceStore @@ -94,6 +95,7 @@ class Database( TaskStore, TaskStatusStore, PlanStore, + ReviewStore, NotificationStore, SourceStore, CronStore, diff --git a/nerve/db/migrations/v045_code_reviews.py b/nerve/db/migrations/v045_code_reviews.py new file mode 100644 index 00000000..4c9c10d4 --- /dev/null +++ b/nerve/db/migrations/v045_code_reviews.py @@ -0,0 +1,70 @@ +"""V45: Local code-review panel — reviews, line-anchored threads, comments. + +Backs the ``code_review`` feature: a browser panel for reviewing on-disk git +worktrees before anything is committed/pushed, with comment threads anchored +to a file + line range that route into a Nerve session and back. + +Purely additive — three new tables, no changes to existing schema — so this +migration is safe to apply to an existing database and trivially reversible by +dropping the tables. +""" + +from __future__ import annotations + +import logging + +import aiosqlite + +logger = logging.getLogger(__name__) + + +async def up(db: aiosqlite.Connection) -> None: + await db.executescript( + """ + CREATE TABLE IF NOT EXISTS code_reviews ( + id TEXT PRIMARY KEY, + title TEXT NOT NULL DEFAULT '', + repo_root TEXT NOT NULL, + worktree TEXT NOT NULL, + branch TEXT, + base_ref TEXT NOT NULL DEFAULT 'HEAD', + target_session_id TEXT, + created_by TEXT NOT NULL DEFAULT 'human', -- 'human' | 'agent' + status TEXT NOT NULL DEFAULT 'open', -- 'open' | 'resolved' + created_at TEXT NOT NULL DEFAULT (datetime('now')), + updated_at TEXT NOT NULL DEFAULT (datetime('now')) + ); + CREATE INDEX IF NOT EXISTS idx_code_reviews_status + ON code_reviews(status); + CREATE INDEX IF NOT EXISTS idx_code_reviews_session + ON code_reviews(target_session_id); + + CREATE TABLE IF NOT EXISTS code_review_threads ( + id TEXT PRIMARY KEY, + review_id TEXT NOT NULL REFERENCES code_reviews(id) + ON DELETE CASCADE ON UPDATE CASCADE, + file_path TEXT NOT NULL, -- repo-relative + side TEXT NOT NULL DEFAULT 'new', -- 'new' | 'old' + line_start INTEGER, + line_end INTEGER, + anchor_snippet TEXT, -- line text at creation + status TEXT NOT NULL DEFAULT 'open', -- 'open' | 'answered' | 'resolved' + created_at TEXT NOT NULL DEFAULT (datetime('now')), + updated_at TEXT NOT NULL DEFAULT (datetime('now')) + ); + CREATE INDEX IF NOT EXISTS idx_code_review_threads_review + ON code_review_threads(review_id); + + CREATE TABLE IF NOT EXISTS code_review_comments ( + id TEXT PRIMARY KEY, + thread_id TEXT NOT NULL REFERENCES code_review_threads(id) + ON DELETE CASCADE ON UPDATE CASCADE, + author TEXT NOT NULL, -- 'human' | 'agent' + body TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (datetime('now')) + ); + CREATE INDEX IF NOT EXISTS idx_code_review_comments_thread + ON code_review_comments(thread_id); + """ + ) + logger.info("v040: created code_reviews, code_review_threads, code_review_comments") diff --git a/nerve/db/reviews.py b/nerve/db/reviews.py new file mode 100644 index 00000000..a32221b6 --- /dev/null +++ b/nerve/db/reviews.py @@ -0,0 +1,211 @@ +"""Code-review data access methods. + +Backs the ``code_review`` panel: reviews own line-anchored threads, threads own +comments. IDs are generated here (short uuid hex) so callers stay simple. All +writes go through the shared ``_write`` / ``_atomic`` helpers on +:class:`nerve.db.base.Database`. +""" + +from __future__ import annotations + +import uuid +from datetime import datetime, timezone + + +def _now() -> str: + return datetime.now(timezone.utc).isoformat() + + +def _new_id() -> str: + return uuid.uuid4().hex[:12] + + +class ReviewStore: + """Mixin providing code-review CRUD operations.""" + + # -- Reviews ------------------------------------------------------------ + + async def create_review( + self, + *, + repo_root: str, + worktree: str, + branch: str | None = None, + base_ref: str = "HEAD", + target_session_id: str | None = None, + created_by: str = "human", + title: str = "", + ) -> dict: + review_id = _new_id() + await self._write( + """INSERT INTO code_reviews + (id, title, repo_root, worktree, branch, base_ref, + target_session_id, created_by, status) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'open')""", + (review_id, title, repo_root, worktree, branch, base_ref, + target_session_id, created_by), + ) + review = await self.get_review(review_id) + assert review is not None + return review + + async def get_review(self, review_id: str) -> dict | None: + async with self.db.execute( + "SELECT * FROM code_reviews WHERE id = ?", (review_id,), + ) as cursor: + row = await cursor.fetchone() + return dict(row) if row else None + + async def list_reviews( + self, status: str | None = None, target_session_id: str | None = None, limit: int = 100, + ) -> list[dict]: + """List reviews (newest first) with open-thread counts, optionally + filtered by status and/or the session they're attached to.""" + conditions = [] + params: list = [] + if status: + conditions.append("r.status = ?") + params.append(status) + if target_session_id: + conditions.append("r.target_session_id = ?") + params.append(target_session_id) + where = f"WHERE {' AND '.join(conditions)}" if conditions else "" + params.append(limit) + async with self.db.execute( + f"""SELECT r.*, + (SELECT COUNT(*) FROM code_review_threads t + WHERE t.review_id = r.id) AS thread_count, + (SELECT COUNT(*) FROM code_review_threads t + WHERE t.review_id = r.id AND t.status = 'open') AS open_thread_count + FROM code_reviews r + {where} + ORDER BY r.updated_at DESC, r.created_at DESC + LIMIT ?""", + tuple(params), + ) as cursor: + return [dict(row) async for row in cursor] + + async def get_review_full(self, review_id: str) -> dict | None: + """Return the review with its threads, each carrying its comments.""" + review = await self.get_review(review_id) + if review is None: + return None + threads = await self.list_threads(review_id) + for thread in threads: + thread["comments"] = await self.list_comments(thread["id"]) + review["threads"] = threads + return review + + async def update_review(self, review_id: str, **fields) -> None: + fields = {k: v for k, v in fields.items() if k in {"title", "status", "target_session_id"}} + if not fields: + return + fields["updated_at"] = _now() + sets = ", ".join(f"{k} = ?" for k in fields) + vals = list(fields.values()) + vals.append(review_id) + await self._write(f"UPDATE code_reviews SET {sets} WHERE id = ?", tuple(vals)) + + async def delete_review(self, review_id: str) -> None: + await self._write("DELETE FROM code_reviews WHERE id = ?", (review_id,)) + + # -- Threads ------------------------------------------------------------ + + async def add_thread( + self, + *, + review_id: str, + file_path: str, + side: str = "new", + line_start: int | None = None, + line_end: int | None = None, + anchor_snippet: str | None = None, + ) -> dict: + thread_id = _new_id() + now = _now() + async with self._atomic(): + await self.db.execute( + """INSERT INTO code_review_threads + (id, review_id, file_path, side, line_start, line_end, + anchor_snippet, status) + VALUES (?, ?, ?, ?, ?, ?, ?, 'open')""", + (thread_id, review_id, file_path, side, line_start, line_end, anchor_snippet), + ) + await self.db.execute( + "UPDATE code_reviews SET updated_at = ? WHERE id = ?", (now, review_id), + ) + thread = await self.get_thread(thread_id) + assert thread is not None + return thread + + async def get_thread(self, thread_id: str) -> dict | None: + async with self.db.execute( + "SELECT * FROM code_review_threads WHERE id = ?", (thread_id,), + ) as cursor: + row = await cursor.fetchone() + return dict(row) if row else None + + async def list_threads(self, review_id: str) -> list[dict]: + async with self.db.execute( + """SELECT * FROM code_review_threads + WHERE review_id = ? + ORDER BY created_at ASC""", + (review_id,), + ) as cursor: + return [dict(row) async for row in cursor] + + async def set_thread_status(self, thread_id: str, status: str) -> None: + await self._write( + "UPDATE code_review_threads SET status = ?, updated_at = ? WHERE id = ?", + (status, _now(), thread_id), + ) + + # -- Comments ----------------------------------------------------------- + + async def add_comment( + self, + *, + thread_id: str, + author: str, + body: str, + thread_status: str | None = None, + ) -> dict: + """Append a comment, bump the thread + its review, and optionally set + the thread status (e.g. 'answered' when the agent replies).""" + comment_id = _new_id() + now = _now() + async with self._atomic(): + await self.db.execute( + """INSERT INTO code_review_comments (id, thread_id, author, body) + VALUES (?, ?, ?, ?)""", + (comment_id, thread_id, author, body), + ) + if thread_status is not None: + await self.db.execute( + "UPDATE code_review_threads SET status = ?, updated_at = ? WHERE id = ?", + (thread_status, now, thread_id), + ) + else: + await self.db.execute( + "UPDATE code_review_threads SET updated_at = ? WHERE id = ?", + (now, thread_id), + ) + await self.db.execute( + """UPDATE code_reviews SET updated_at = ? + WHERE id = (SELECT review_id FROM code_review_threads WHERE id = ?)""", + (now, thread_id), + ) + async with self.db.execute( + "SELECT * FROM code_review_comments WHERE id = ?", (comment_id,), + ) as cursor: + row = await cursor.fetchone() + return dict(row) if row else {"id": comment_id} + + async def list_comments(self, thread_id: str) -> list[dict]: + async with self.db.execute( + """SELECT * FROM code_review_comments + WHERE thread_id = ? + ORDER BY created_at ASC""", + (thread_id,), + ) as cursor: + return [dict(row) async for row in cursor] diff --git a/nerve/gateway/gitreview.py b/nerve/gateway/gitreview.py new file mode 100644 index 00000000..b96f1264 --- /dev/null +++ b/nerve/gateway/gitreview.py @@ -0,0 +1,164 @@ +"""Git helpers for the code-review panel. + +Enumerate the git worktrees of configured repo roots, list working-tree +changes against a base ref, and read file content at a ref or from the working +tree. Every path is confined to a configured repo root via +:func:`resolve_within_repos` (path-traversal / symlink-escape guard). + +All functions here are synchronous ``subprocess`` wrappers — call them from +routes via ``asyncio.to_thread`` so git never blocks the event loop. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + + +class RepoAccessError(Exception): + """Raised for an invalid repo/worktree/path request (maps to HTTP 400).""" + + +def _git(cwd: Path, *args: str, timeout: int = 20) -> str: + proc = subprocess.run( + ["git", "-C", str(cwd), *args], + capture_output=True, + text=True, + timeout=timeout, + ) + if proc.returncode != 0: + raise RepoAccessError(proc.stderr.strip() or f"git {' '.join(args)} failed") + return proc.stdout + + +def list_worktrees_sync(root: Path) -> list[dict]: + """Parse ``git worktree list --porcelain`` for one repo root.""" + out = _git(root, "worktree", "list", "--porcelain") + worktrees: list[dict] = [] + cur: dict = {} + for line in out.splitlines(): + if not line.strip(): + if cur: + worktrees.append(cur) + cur = {} + continue + key, _, val = line.partition(" ") + if key == "worktree": + cur["path"] = val + elif key == "branch": + cur["branch"] = val.replace("refs/heads/", "") + elif key == "HEAD": + cur["head"] = val[:12] + elif key == "detached": + cur["branch"] = "(detached)" + if cur: + worktrees.append(cur) + return worktrees + + +def _all_worktree_paths(repos: list[str]) -> dict[Path, Path]: + """Map every configured worktree's resolved path -> its owning repo root. + + Silently skips roots that aren't valid git repos so one bad config entry + doesn't break the whole panel. + """ + mapping: dict[Path, Path] = {} + for r in repos: + root = Path(r).expanduser().resolve() + try: + for w in list_worktrees_sync(root): + mapping[Path(w["path"]).resolve()] = root + except (RepoAccessError, subprocess.SubprocessError, OSError): + continue + return mapping + + +def resolve_within_repos( + worktree: str, + repos: list[str], + path: str | None = None, +) -> tuple[Path, Path, Path | None]: + """Validate a worktree (+ optional file path) against configured repos. + + Returns ``(worktree_path, repo_root, target_path_or_None)``. + + - ``worktree`` must be a real git worktree of one configured repo root. + - ``path`` (repo-relative) must resolve to a location inside that worktree + — after symlink resolution — so ``..`` and symlink escapes are rejected. + """ + wt = Path(worktree).expanduser().resolve() + valid = _all_worktree_paths(repos) + root = valid.get(wt) + if root is None: + raise RepoAccessError(f"worktree is not part of a configured repo: {worktree}") + + if path is None: + return wt, root, None + + candidate = Path(path) + target = (candidate if candidate.is_absolute() else wt / candidate).resolve() + if target != wt and not target.is_relative_to(wt): + raise RepoAccessError("path escapes the worktree") + return wt, root, target + + +def changed_files_sync(worktree: Path, base: str = "HEAD") -> list[dict]: + """List files that differ between ``base`` and the working tree. + + Covers tracked changes (``git diff base``, i.e. staged + unstaged) plus + untracked files. Each entry: ``{path, status, additions, deletions}``. + """ + files: dict[str, dict] = {} + + name_status = _git(worktree, "diff", "--name-status", base, "--") + for line in name_status.splitlines(): + if not line.strip(): + continue + parts = line.split("\t") + code = parts[0] + if code.startswith("R") and len(parts) >= 3: # rename: "R100\told\tnew" + path, status = parts[2], "renamed" + elif len(parts) >= 2: + path = parts[1] + status = {"A": "created", "M": "modified", "D": "deleted"}.get(code[0], "modified") + else: + continue + files[path] = {"path": path, "status": status, "additions": 0, "deletions": 0} + + numstat = _git(worktree, "diff", "--numstat", base, "--") + for line in numstat.splitlines(): + cols = line.split("\t") + if len(cols) < 3: + continue + adds, dels, path = cols[0], cols[1], cols[2] + entry = files.get(path) + if entry is not None: + entry["additions"] = 0 if adds == "-" else int(adds or 0) + entry["deletions"] = 0 if dels == "-" else int(dels or 0) + + untracked = _git(worktree, "ls-files", "--others", "--exclude-standard") + for path in untracked.splitlines(): + if path.strip(): + files.setdefault(path, {"path": path, "status": "created", "additions": 0, "deletions": 0}) + + return sorted(files.values(), key=lambda f: f["path"]) + + +def file_at_ref_sync(worktree: Path, ref: str, path: str) -> str | None: + """Content of ``path`` at ``ref`` (repo-relative); None if absent there.""" + try: + return _git(worktree, "show", f"{ref}:{path}") + except RepoAccessError: + return None + + +def read_working_file_sync(target: Path | None, max_bytes: int) -> str | None: + """Read the working-tree file; None if missing; raise if too large/binary.""" + if target is None or not target.exists() or not target.is_file(): + return None + if target.stat().st_size > max_bytes: + raise RepoAccessError(f"file exceeds max_file_bytes ({max_bytes})") + data = target.read_bytes() + if b"\x00" in data[:8192]: + raise RepoAccessError("binary file") + return data.decode("utf-8", errors="replace") diff --git a/nerve/gateway/routes/__init__.py b/nerve/gateway/routes/__init__.py index 55436e44..2fc0f9df 100644 --- a/nerve/gateway/routes/__init__.py +++ b/nerve/gateway/routes/__init__.py @@ -36,6 +36,7 @@ workflow_runs, review_loops, config, + reviews, ) __all__ = [ @@ -69,4 +70,5 @@ def register_all_routes() -> APIRouter: router.include_router(workflow_runs.router) router.include_router(review_loops.router) router.include_router(config.router) + router.include_router(reviews.router) return router diff --git a/nerve/gateway/routes/reviews.py b/nerve/gateway/routes/reviews.py new file mode 100644 index 00000000..6a78ea67 --- /dev/null +++ b/nerve/gateway/routes/reviews.py @@ -0,0 +1,349 @@ +"""Code-review panel routes. + +Feature-gated by ``config.code_review.enabled``. Serves git working-tree diffs +and file contents for configured repo worktrees, and manages line-anchored +review threads whose comments route into a Nerve session (and back). Every +route requires the standard web-UI auth; file access is confined to configured +repo roots by :func:`nerve.gateway.gitreview.resolve_within_repos`. +""" + +from __future__ import annotations + +import asyncio +import logging +import uuid +from pathlib import Path + +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel + +from nerve.config import get_config +from nerve.gateway import gitreview +from nerve.gateway.auth import require_auth +from nerve.gateway.diff import compute_file_diff +from nerve.gateway.gitreview import RepoAccessError +from nerve.gateway.routes._deps import get_deps + +logger = logging.getLogger(__name__) + +router = APIRouter() + + +def _cfg(): + cfg = get_config().code_review + if not cfg.enabled: + raise HTTPException(status_code=404, detail="Code review is not enabled") + return cfg + + +def _mint_session_id() -> str: + return str(uuid.uuid4())[:8] + + +# --- Comment → session injection ------------------------------------------- +# +# A human comment is delivered to the review's target session by running a +# turn in that session — exactly what POST /api/chat does, but detached so the +# HTTP response returns immediately (engine.run blocks for the whole turn). A +# per-session lock serializes our injects so two comments can't start +# overlapping turns on the same session. + +_inject_locks: dict[str, asyncio.Lock] = {} + + +def _lock_for(session_id: str) -> asyncio.Lock: + lock = _inject_locks.get(session_id) + if lock is None: + lock = asyncio.Lock() + _inject_locks[session_id] = lock + return lock + + +async def _run_inject(session_id: str, message: str) -> None: + deps = get_deps() + async with _lock_for(session_id): + try: + await deps.engine.run( + session_id=session_id, + user_message=message, + source="web", + channel="web", + ) + except Exception: + logger.exception("code-review inject into session %s failed", session_id) + + +def _format_comment_message(review: dict, thread: dict, body: str) -> str: + loc = thread["file_path"] + ls, le = thread.get("line_start"), thread.get("line_end") + if ls and le and le != ls: + loc += f":{ls}-{le}" + elif ls: + loc += f":{ls}" + header = ( + f'Review {review["id"]} "{review.get("title") or "(untitled)"}" — ' + f'{review["worktree"]}' + ) + if review.get("branch"): + header += f' (branch {review["branch"]})' + lines = [ + "[code-review comment · from the reviewer — automated delivery, not a chat message]", + header, + f'File: {loc} ({thread.get("side", "new")} side)', + ] + if thread.get("anchor_snippet"): + lines.append(f'> {thread["anchor_snippet"]}') + lines += [ + "", + "The reviewer wrote:", + body, + "", + f"(review {review['id']}, thread {thread['id']}) — reply in this chat to " + "discuss. To post your answer back onto the review thread so it appears " + "inline in the Code Review panel and marks the thread answered, use your " + "code-review reply tool with that review id and thread id.", + ] + return "\n".join(lines) + + +async def _ensure_target(review: dict) -> str: + """Return the review's target session id, minting + persisting one if unset.""" + target = review.get("target_session_id") + if not target: + target = _mint_session_id() + await get_deps().db.update_review(review["id"], target_session_id=target) + review["target_session_id"] = target + return target + + +# --- Request models --------------------------------------------------------- + +class ReviewCreate(BaseModel): + worktree: str + branch: str | None = None + base_ref: str = "HEAD" + target_session_id: str | None = None + created_by: str = "human" + title: str = "" + + +class ReviewPatch(BaseModel): + title: str | None = None + status: str | None = None + target_session_id: str | None = None + + +class ThreadCreate(BaseModel): + file_path: str + side: str = "new" + line_start: int | None = None + line_end: int | None = None + anchor_snippet: str | None = None + body: str = "" + author: str = "human" + + +class CommentCreate(BaseModel): + body: str + author: str = "human" + + +# --- Git-backed read endpoints --------------------------------------------- + +@router.get("/api/review/repos") +async def review_repos(user: dict = Depends(require_auth)): + cfg = _cfg() + repos = [] + for r in cfg.repos: + root = Path(r).expanduser() + try: + worktrees = await asyncio.to_thread(gitreview.list_worktrees_sync, root.resolve()) + except (RepoAccessError, OSError): + continue + repos.append({"root": str(root), "resolved": str(root.resolve()), "worktrees": worktrees}) + return {"repos": repos} + + +@router.get("/api/review/changed") +async def review_changed(worktree: str, base: str = "HEAD", user: dict = Depends(require_auth)): + cfg = _cfg() + try: + wt, root, _ = await asyncio.to_thread(gitreview.resolve_within_repos, worktree, cfg.repos, None) + files = await asyncio.to_thread(gitreview.changed_files_sync, wt, base) + except RepoAccessError as e: + raise HTTPException(status_code=400, detail=str(e)) + return {"worktree": str(wt), "repo_root": str(root), "base": base, "files": files} + + +@router.get("/api/review/diff") +async def review_diff( + worktree: str, + path: str, + base: str = "HEAD", + context: int = 4, + user: dict = Depends(require_auth), +): + cfg = _cfg() + try: + wt, _root, target = await asyncio.to_thread( + gitreview.resolve_within_repos, worktree, cfg.repos, path, + ) + original = await asyncio.to_thread(gitreview.file_at_ref_sync, wt, base, path) + current = await asyncio.to_thread(gitreview.read_working_file_sync, target, cfg.max_file_bytes) + except RepoAccessError as e: + raise HTTPException(status_code=400, detail=str(e)) + diff = await asyncio.to_thread(compute_file_diff, original, current, path, context, None) + return diff + + +@router.get("/api/review/file") +async def review_file(worktree: str, path: str, user: dict = Depends(require_auth)): + cfg = _cfg() + try: + _wt, _root, target = await asyncio.to_thread( + gitreview.resolve_within_repos, worktree, cfg.repos, path, + ) + except RepoAccessError as e: + raise HTTPException(status_code=400, detail=str(e)) + try: + content = await asyncio.to_thread(gitreview.read_working_file_sync, target, cfg.max_file_bytes) + return {"path": path, "content": content, "binary": False, "too_large": False} + except RepoAccessError as e: + msg = str(e) + return { + "path": path, + "content": None, + "binary": "binary" in msg, + "too_large": "max_file_bytes" in msg, + } + + +# --- Review / thread / comment endpoints ----------------------------------- + +@router.get("/api/reviews") +async def list_reviews( + status: str | None = None, + session: str | None = None, + user: dict = Depends(require_auth), +): + _cfg() + reviews = await get_deps().db.list_reviews(status=status, target_session_id=session) + return {"reviews": reviews} + + +@router.post("/api/reviews") +async def create_review(req: ReviewCreate, user: dict = Depends(require_auth)): + cfg = _cfg() + try: + wt, root, _ = await asyncio.to_thread(gitreview.resolve_within_repos, req.worktree, cfg.repos, None) + except RepoAccessError as e: + raise HTTPException(status_code=400, detail=str(e)) + + branch = req.branch + if not branch: + try: + for w in await asyncio.to_thread(gitreview.list_worktrees_sync, root): + if Path(w["path"]).resolve() == wt: + branch = w.get("branch") + break + except RepoAccessError: + branch = None + + review = await get_deps().db.create_review( + repo_root=str(root), + worktree=str(wt), + branch=branch, + base_ref=req.base_ref, + target_session_id=req.target_session_id, + created_by=req.created_by, + title=req.title, + ) + return review + + +@router.get("/api/reviews/{review_id}") +async def get_review(review_id: str, user: dict = Depends(require_auth)): + _cfg() + review = await get_deps().db.get_review_full(review_id) + if review is None: + raise HTTPException(status_code=404, detail="Review not found") + return review + + +@router.patch("/api/reviews/{review_id}") +async def patch_review(review_id: str, req: ReviewPatch, user: dict = Depends(require_auth)): + _cfg() + db = get_deps().db + if await db.get_review(review_id) is None: + raise HTTPException(status_code=404, detail="Review not found") + fields = {k: v for k, v in req.model_dump().items() if v is not None} + await db.update_review(review_id, **fields) + return await db.get_review_full(review_id) + + +@router.delete("/api/reviews/{review_id}") +async def delete_review(review_id: str, user: dict = Depends(require_auth)): + _cfg() + await get_deps().db.delete_review(review_id) + return {"deleted": True} + + +@router.post("/api/reviews/{review_id}/threads") +async def create_thread(review_id: str, req: ThreadCreate, user: dict = Depends(require_auth)): + _cfg() + db = get_deps().db + review = await db.get_review(review_id) + if review is None: + raise HTTPException(status_code=404, detail="Review not found") + + thread = await db.add_thread( + review_id=review_id, + file_path=req.file_path, + side=req.side, + line_start=req.line_start, + line_end=req.line_end, + anchor_snippet=req.anchor_snippet, + ) + if req.body: + await db.add_comment(thread_id=thread["id"], author=req.author, body=req.body) + + target = None + if req.author == "human": + target = await _ensure_target(review) + asyncio.create_task(_run_inject(target, _format_comment_message(review, thread, req.body or "(no text)"))) + + thread["comments"] = await db.list_comments(thread["id"]) + return {"thread": thread, "target_session_id": target} + + +@router.post("/api/reviews/{review_id}/threads/{thread_id}/comments") +async def add_comment(review_id: str, thread_id: str, req: CommentCreate, user: dict = Depends(require_auth)): + _cfg() + db = get_deps().db + review = await db.get_review(review_id) + thread = await db.get_thread(thread_id) + if review is None or thread is None or thread["review_id"] != review_id: + raise HTTPException(status_code=404, detail="Review or thread not found") + + thread_status = "answered" if req.author == "agent" else None + comment = await db.add_comment( + thread_id=thread_id, author=req.author, body=req.body, thread_status=thread_status, + ) + + target = None + if req.author == "human": + target = await _ensure_target(review) + asyncio.create_task(_run_inject(target, _format_comment_message(review, thread, req.body))) + + return {"comment": comment, "target_session_id": target} + + +@router.post("/api/reviews/{review_id}/threads/{thread_id}/resolve") +async def resolve_thread(review_id: str, thread_id: str, user: dict = Depends(require_auth)): + _cfg() + db = get_deps().db + thread = await db.get_thread(thread_id) + if thread is None or thread["review_id"] != review_id: + raise HTTPException(status_code=404, detail="Thread not found") + await db.set_thread_status(thread_id, "resolved") + return {"resolved": True} diff --git a/tests/test_reviews.py b/tests/test_reviews.py new file mode 100644 index 00000000..d22cab7f --- /dev/null +++ b/tests/test_reviews.py @@ -0,0 +1,266 @@ +"""Tests for the code-review panel: ReviewStore, git helpers, and routes.""" + +from __future__ import annotations + +import asyncio +import subprocess +from types import SimpleNamespace + +import pytest +import pytest_asyncio + +from nerve.db import Database +from nerve.gateway import gitreview + + +# --------------------------------------------------------------------------- # +# Helpers # +# --------------------------------------------------------------------------- # + +def _git(cwd, *args): + subprocess.run(["git", "-C", str(cwd), *args], check=True, capture_output=True, text=True) + + +def _make_repo(path): + """A git repo with a committed file, an uncommitted modification, and an + untracked file.""" + path.mkdir(parents=True, exist_ok=True) + _git(path, "init", "-q") + _git(path, "config", "user.email", "t@example.com") + _git(path, "config", "user.name", "Test") + (path / "a.txt").write_text("line1\nline2\nline3\n") + (path / "keep.txt").write_text("unchanged\n") + _git(path, "add", "-A") + _git(path, "commit", "-q", "-m", "init") + (path / "a.txt").write_text("line1\nCHANGED\nline3\nline4\n") # modified + (path / "new.txt").write_text("brand new\n") # untracked + return path + + +class FakeEngine: + """Records engine.run() calls (the comment-inject path).""" + + def __init__(self): + self.runs: list[dict] = [] + self.run_event = asyncio.Event() + + async def run(self, *, session_id, user_message, source, channel): + self.runs.append({ + "session_id": session_id, "user_message": user_message, + "source": source, "channel": channel, + }) + self.run_event.set() + return "ok" + + def is_session_running(self, session_id): # pragma: no cover - unused here + return False + + +# --------------------------------------------------------------------------- # +# ReviewStore # +# --------------------------------------------------------------------------- # + +@pytest.mark.asyncio +class TestReviewStore: + async def test_migration_applied(self, db: Database): + from nerve.db.base import SCHEMA_VERSION + assert SCHEMA_VERSION >= 40 + + async def test_review_thread_comment_lifecycle(self, db: Database): + review = await db.create_review( + repo_root="/r", worktree="/r/wt", branch="feature", + base_ref="HEAD", created_by="agent", title="my review", + target_session_id="sess1234", + ) + rid = review["id"] + + thread = await db.add_thread( + review_id=rid, file_path="a.txt", side="new", + line_start=2, line_end=3, anchor_snippet="CHANGED", + ) + await db.add_comment(thread_id=thread["id"], author="human", body="why?") + await db.add_comment(thread_id=thread["id"], author="agent", body="because", + thread_status="answered") + + full = await db.get_review_full(rid) + assert full["title"] == "my review" + assert len(full["threads"]) == 1 + t = full["threads"][0] + assert t["status"] == "answered" + assert t["line_start"] == 2 and t["line_end"] == 3 + assert [c["author"] for c in t["comments"]] == ["human", "agent"] + + listed = await db.list_reviews() + assert listed[0]["thread_count"] == 1 + assert listed[0]["open_thread_count"] == 0 # thread is 'answered', not 'open' + + await db.set_thread_status(t["id"], "resolved") + assert (await db.get_thread(t["id"]))["status"] == "resolved" + + async def test_status_filter_and_delete(self, db: Database): + r = await db.create_review(repo_root="/r", worktree="/r/wt") + await db.update_review(r["id"], status="resolved") + assert [x["id"] for x in await db.list_reviews(status="open")] == [] + assert [x["id"] for x in await db.list_reviews(status="resolved")] == [r["id"]] + await db.delete_review(r["id"]) + assert await db.get_review(r["id"]) is None + + async def test_filter_by_session(self, db: Database): + a = await db.create_review(repo_root="/r", worktree="/r/wt", target_session_id="sessAAAA") + b = await db.create_review(repo_root="/r", worktree="/r/wt2", target_session_id="sessBBBB") + assert {x["id"] for x in await db.list_reviews(target_session_id="sessAAAA")} == {a["id"]} + assert {x["id"] for x in await db.list_reviews(target_session_id="sessBBBB")} == {b["id"]} + assert {x["id"] for x in await db.list_reviews()} == {a["id"], b["id"]} + + +# --------------------------------------------------------------------------- # +# Git helpers # +# --------------------------------------------------------------------------- # + +class TestGitReview: + def test_changed_files(self, tmp_path): + repo = _make_repo(tmp_path / "repo") + by_path = {f["path"]: f for f in gitreview.changed_files_sync(repo, "HEAD")} + assert by_path["a.txt"]["status"] == "modified" + assert by_path["a.txt"]["additions"] >= 1 and by_path["a.txt"]["deletions"] >= 1 + assert by_path["new.txt"]["status"] == "created" + assert "keep.txt" not in by_path + + def test_file_at_ref_and_working(self, tmp_path): + repo = _make_repo(tmp_path / "repo") + _wt, _root, target = gitreview.resolve_within_repos(str(repo), [str(repo)], "a.txt") + original = gitreview.file_at_ref_sync(repo, "HEAD", "a.txt") + current = gitreview.read_working_file_sync(target, 1_000_000) + assert original == "line1\nline2\nline3\n" + assert current == "line1\nCHANGED\nline3\nline4\n" + # A file that doesn't exist at HEAD (new file) → None original + assert gitreview.file_at_ref_sync(repo, "HEAD", "new.txt") is None + + def test_resolve_rejects_traversal_and_foreign_worktree(self, tmp_path): + repo = _make_repo(tmp_path / "repo") + # traversal escape + with pytest.raises(gitreview.RepoAccessError): + gitreview.resolve_within_repos(str(repo), [str(repo)], "../../etc/passwd") + # a path not registered as a worktree of any configured repo + outside = tmp_path / "outside" + outside.mkdir() + with pytest.raises(gitreview.RepoAccessError): + gitreview.resolve_within_repos(str(outside), [str(repo)], None) + + def test_read_working_file_limits(self, tmp_path): + repo = _make_repo(tmp_path / "repo") + _wt, _root, target = gitreview.resolve_within_repos(str(repo), [str(repo)], "a.txt") + with pytest.raises(gitreview.RepoAccessError): + gitreview.read_working_file_sync(target, 2) # smaller than the file + + +# --------------------------------------------------------------------------- # +# Routes (TestClient + fake engine, auth bypassed) # +# --------------------------------------------------------------------------- # + +@pytest.mark.asyncio +class TestReviewRoutes: + @pytest_asyncio.fixture + async def app_setup(self, db: Database, tmp_path): + from fastapi import FastAPI + from fastapi.testclient import TestClient + + import nerve.config as cfg_mod + from nerve.config import NerveConfig + from nerve.gateway.routes._deps import init_deps + from nerve.gateway.routes.reviews import router as reviews_router + + repo = _make_repo(tmp_path / "repo") + + cfg = NerveConfig() + cfg.workspace = tmp_path + cfg.auth.jwt_secret = "" # require_auth becomes a no-op + cfg.code_review.enabled = True + cfg.code_review.repos = [str(repo)] + cfg_mod._config = cfg + + engine = FakeEngine() + init_deps(engine=engine, db=db) # type: ignore[arg-type] + + app = FastAPI() + app.include_router(reviews_router) + client = TestClient(app) + + yield SimpleNamespace(client=client, engine=engine, db=db, repo=repo, cfg=cfg) + + cfg_mod._config = None + + async def test_repos_and_changed(self, app_setup): + c = app_setup.client + repos = c.get("/api/review/repos").json()["repos"] + assert len(repos) == 1 + assert any(str(app_setup.repo) == wt["path"] or str(app_setup.repo.resolve()) == wt["path"] + for wt in repos[0]["worktrees"]) + + changed = c.get("/api/review/changed", params={"worktree": str(app_setup.repo)}).json() + names = {f["path"] for f in changed["files"]} + assert "a.txt" in names and "new.txt" in names + + async def test_diff_and_file(self, app_setup): + c = app_setup.client + d = c.get("/api/review/diff", + params={"worktree": str(app_setup.repo), "path": "a.txt"}).json() + assert d["status"] == "modified" + assert d["patch"] + assert any(ln["type"] == "addition" for h in d["hunks"] for ln in h["lines"]) + + f = c.get("/api/review/file", + params={"worktree": str(app_setup.repo), "path": "a.txt"}).json() + assert f["binary"] is False and "CHANGED" in f["content"] + + async def test_path_traversal_returns_400(self, app_setup): + r = app_setup.client.get( + "/api/review/diff", + params={"worktree": str(app_setup.repo), "path": "../../../etc/passwd"}, + ) + assert r.status_code == 400 + + async def test_disabled_returns_404(self, app_setup): + app_setup.cfg.code_review.enabled = False + try: + assert app_setup.client.get("/api/review/repos").status_code == 404 + finally: + app_setup.cfg.code_review.enabled = True + + async def test_human_comment_injects_agent_reply_does_not(self, app_setup): + c, engine = app_setup.client, app_setup.engine + + review = c.post("/api/reviews", json={ + "worktree": str(app_setup.repo), "title": "t", "created_by": "agent", + }).json() + rid = review["id"] + assert review["branch"] # derived from the worktree + + # Human thread → mints a target session and injects. + resp = c.post(f"/api/reviews/{rid}/threads", json={ + "file_path": "a.txt", "side": "new", "line_start": 2, "line_end": 2, + "anchor_snippet": "CHANGED", "body": "why change this?", "author": "human", + }) + assert resp.status_code == 200 + data = resp.json() + target = data["target_session_id"] + assert target + tid = data["thread"]["id"] + + await asyncio.wait_for(engine.run_event.wait(), timeout=2.0) + assert len(engine.runs) == 1 + assert engine.runs[0]["session_id"] == target + assert "why change this?" in engine.runs[0]["user_message"] + assert "a.txt:2" in engine.runs[0]["user_message"] + + # Agent reply → marks the thread answered, and does NOT inject. + engine.run_event.clear() + r2 = c.post(f"/api/reviews/{rid}/threads/{tid}/comments", + json={"body": "because Y", "author": "agent"}) + assert r2.status_code == 200 + + full = c.get(f"/api/reviews/{rid}").json() + th = full["threads"][0] + assert th["status"] == "answered" + assert [cm["author"] for cm in th["comments"]] == ["human", "agent"] + assert len(engine.runs) == 1 # no extra inject from the agent reply diff --git a/web/src/api/client.ts b/web/src/api/client.ts index f9a77fca..529115f6 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1,3 +1,8 @@ +import type { FileDiff } from '../types/chat'; +import type { + Review, ReviewThread, ReviewComment, ReviewRepo, ReviewChangedFile, +} from '../types/review'; + const API_BASE = '/api'; export interface TaskStatusDef { @@ -386,6 +391,47 @@ export const api = { getSessionEvents: (id: string, limit = 50) => request<{ events: any[] }>(`/sessions/${id}/events?limit=${limit}`), + // Code review (local git worktree review panel) + reviewRepos: () => request<{ repos: ReviewRepo[] }>('/review/repos'), + reviewChanged: (worktree: string, base = 'HEAD') => + request<{ worktree: string; repo_root: string; base: string; files: ReviewChangedFile[] }>( + `/review/changed?${new URLSearchParams({ worktree, base })}`, + ), + reviewDiff: (worktree: string, path: string, base = 'HEAD') => + request(`/review/diff?${new URLSearchParams({ worktree, path, base })}`), + reviewFile: (worktree: string, path: string) => + request<{ path: string; content: string | null; binary: boolean; too_large: boolean }>( + `/review/file?${new URLSearchParams({ worktree, path })}`, + ), + listReviews: (params?: { status?: string; session?: string }) => { + const qs = new URLSearchParams(); + if (params?.status) qs.set('status', params.status); + if (params?.session) qs.set('session', params.session); + const q = qs.toString(); + return request<{ reviews: Review[] }>(`/reviews${q ? '?' + q : ''}`); + }, + createReview: (data: { + worktree: string; branch?: string | null; base_ref?: string; + target_session_id?: string | null; created_by?: string; title?: string; + }) => request('/reviews', { method: 'POST', body: JSON.stringify(data) }), + getReview: (id: string) => request(`/reviews/${id}`), + patchReview: (id: string, data: { title?: string; status?: string; target_session_id?: string | null }) => + request(`/reviews/${id}`, { method: 'PATCH', body: JSON.stringify(data) }), + deleteReview: (id: string) => + request<{ deleted: boolean }>(`/reviews/${id}`, { method: 'DELETE' }), + addReviewThread: (id: string, data: { + file_path: string; side?: string; line_start?: number | null; line_end?: number | null; + anchor_snippet?: string | null; body?: string; author?: string; + }) => request<{ thread: ReviewThread; target_session_id: string | null }>( + `/reviews/${id}/threads`, { method: 'POST', body: JSON.stringify(data) }, + ), + addReviewComment: (id: string, threadId: string, data: { body: string; author?: string }) => + request<{ comment: ReviewComment; target_session_id: string | null }>( + `/reviews/${id}/threads/${threadId}/comments`, { method: 'POST', body: JSON.stringify(data) }, + ), + resolveReviewThread: (id: string, threadId: string) => + request<{ resolved: boolean }>(`/reviews/${id}/threads/${threadId}/resolve`, { method: 'POST' }), + // Chat (non-streaming) chat: (message: string, sessionId?: string) => request<{ response: string; session_id: string }>('/chat', { diff --git a/web/src/components/Chat/FileChangesPanel.tsx b/web/src/components/Chat/FileChangesPanel.tsx index 2216ca08..53166571 100644 --- a/web/src/components/Chat/FileChangesPanel.tsx +++ b/web/src/components/Chat/FileChangesPanel.tsx @@ -1,36 +1,45 @@ import { useState, useEffect, useRef, lazy, Suspense } from 'react'; -import { ArrowLeft, Eye, FilePlus, FileEdit, FileX, Loader2, RefreshCw, WrapText } from 'lucide-react'; +import { + ArrowLeft, Eye, FilePlus, FileEdit, FileX, Loader2, RefreshCw, WrapText, + Plus, GitBranch, Trash2, X, MessageSquare, FileText, +} from 'lucide-react'; import { useChatStore } from '../../stores/chatStore'; +import { useReviewStore } from '../../stores/reviewStore'; import { api } from '../../api/client'; import { SelectionToolbar } from './SelectionToolbar'; import { MarkdownContent } from './MarkdownContent'; import { MAX_DIFF_LINES } from '../../types/chat'; import type { FileDiff, ModifiedFileSummary } from '../../types/chat'; +import type { ReviewChangedFile } from '../../types/review'; +import { ReviewDiff } from '../Review/ReviewDiff'; // The diff renderer pulls in @pierre/diffs + Shiki — only loaded when a file // diff is actually opened, keeping it off the initial bundle. const DiffView = lazy(() => import('./DiffView').then((m) => ({ default: m.DiffView }))); // ------------------------------------------------------------------ // -// File list view // +// Shared bits // // ------------------------------------------------------------------ // const STATUS_ICON: Record = { created: FilePlus, modified: FileEdit, deleted: FileX, + renamed: FileEdit, }; const STATUS_COLOR: Record = { created: 'text-diff-add', modified: 'text-warning', deleted: 'text-diff-del', + renamed: 'text-hue-blue', }; const STATUS_BADGE: Record = { created: '+', modified: 'M', deleted: 'D', + renamed: 'R', }; function splitPath(shortPath: string): { fileName: string; dirPath: string } { @@ -40,48 +49,43 @@ function splitPath(shortPath: string): { fileName: string; dirPath: string } { return { fileName, dirPath }; } +function basename(p: string): string { + const parts = p.replace(/\/+$/, '').split('/'); + return parts[parts.length - 1] || p; +} + +// ------------------------------------------------------------------ // +// Snapshot "session edits" list item + detail (existing behavior) // +// ------------------------------------------------------------------ // + function FileCard({ file, onClick }: { file: ModifiedFileSummary; onClick: () => void }) { const { fileName, dirPath } = splitPath(file.short_path); const Icon = STATUS_ICON[file.status] || FileEdit; const color = STATUS_COLOR[file.status] || 'text-text-muted'; const badge = STATUS_BADGE[file.status] || '?'; - return ( ); } -// ------------------------------------------------------------------ // -// Detail view (loads diff on demand) // -// ------------------------------------------------------------------ // - -// Persisted line-wrap preference for diff inspection (shared across sessions). const WRAP_STORAGE_KEY = 'nerve_diff_wrap'; function FileDetailView({ file, onBack }: { file: ModifiedFileSummary; onBack: () => void }) { @@ -90,7 +94,6 @@ function FileDetailView({ file, onBack }: { file: ModifiedFileSummary; onBack: ( const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [wrap, setWrap] = useState(() => localStorage.getItem(WRAP_STORAGE_KEY) === 'true'); - // Markdown files only: toggle between the raw diff and a rendered preview. const [preview, setPreview] = useState(false); const containerRef = useRef(null); @@ -112,89 +115,48 @@ function FileDetailView({ file, onBack }: { file: ModifiedFileSummary; onBack: ( return () => { cancelled = true; }; }, [activeSession, file.path]); - // Empty string is a valid (empty) markdown doc — check against null/undefined. - const canPreview = diff?.markdown_content != null; - const { fileName } = splitPath(file.short_path); const color = STATUS_COLOR[file.status] || 'text-text-muted'; return (
- {/* Detail header */}
- {fileName}
- {diff?.stats && diff.stats.additions > 0 && ( - +{diff.stats.additions} - )} - {diff?.stats && diff.stats.deletions > 0 && ( - −{diff.stats.deletions} - )} + {diff?.stats && diff.stats.additions > 0 && +{diff.stats.additions}} + {diff?.stats && diff.stats.deletions > 0 && −{diff.stats.deletions}}
- {canPreview && ( - )} -
-
- {file.short_path} -
- - {/* Diff content */} +
{file.short_path}
- {loading && ( -
- Loading diff... -
- )} - {error && ( -
Failed to load diff: {error}
- )} + {loading &&
Loading diff...
} + {error &&
Failed to load diff: {error}
} {diff && !loading && ( preview && diff.markdown_content != null ? (
- {diff.markdown_truncated && ( -
- Preview truncated at {MAX_DIFF_LINES} lines -
- )} + {diff.markdown_truncated &&
Preview truncated at {MAX_DIFF_LINES} lines
}
) : ( - - Loading diff… -
- } - > + Loading diff…
}> ) @@ -205,20 +167,205 @@ function FileDetailView({ file, onBack }: { file: ModifiedFileSummary; onBack: ( } // ------------------------------------------------------------------ // -// Main panel component // +// Attach-worktree picker // +// ------------------------------------------------------------------ // + +function AttachPicker({ onClose }: { onClose: () => void }) { + const repos = useReviewStore(s => s.repos); + const loadRepos = useReviewStore(s => s.loadRepos); + const attach = useReviewStore(s => s.attach); + const busy = useReviewStore(s => s.busy); + const [sel, setSel] = useState(''); // "worktreePath branch" + const [base, setBase] = useState('HEAD'); + + useEffect(() => { loadRepos(); }, [loadRepos]); + + const options: { path: string; branch: string | null; repo: string }[] = []; + for (const r of repos) for (const wt of r.worktrees) options.push({ path: wt.path, branch: wt.branch ?? null, repo: r.root }); + + const doAttach = () => { + if (!sel) return; + const [path, branch] = sel.split(' '); + attach(path, branch || null, base.trim() || 'HEAD').then(onClose); + }; + + return ( +
+
+ Attach a worktree + +
+ +
+ + setBase(e.target.value)} + className="w-24 bg-bg border border-border rounded px-1.5 py-0.5 text-[12px] text-text" /> + +
+
+ ); +} + +// ------------------------------------------------------------------ // +// Attached-worktree views (git diff + line comments) // +// ------------------------------------------------------------------ // + +function ChangedFileRow({ file, onClick }: { file: ReviewChangedFile; onClick: () => void }) { + const { fileName, dirPath } = splitPath(file.path); + const color = STATUS_COLOR[file.status] || 'text-text-muted'; + const badge = STATUS_BADGE[file.status] || '?'; + return ( + + ); +} + +// The active worktree's changed-files list (+ header with base + refresh). +function ReviewWorktreeView() { + const r = useReviewStore(s => s.activeReview)!; + const changed = useReviewStore(s => s.changed); + const loading = useReviewStore(s => s.loadingChanged); + const closeReview = useReviewStore(s => s.closeReview); + const selectFile = useReviewStore(s => s.selectFile); + const selectReview = useReviewStore(s => s.selectReview); + + return ( +
+
+ + + {r.branch || basename(r.worktree)} + vs {r.base_ref} + +
+
{r.worktree}
+
+ {loading &&
Loading changes…
} + {!loading && changed.length === 0 &&
No changes vs {r.base_ref}.
} + {changed.map(f => selectFile(f.path)} />)} +
+
+ ); +} + +// A single file's git diff with inline, line-anchored comment threads. +function ReviewFileDetail() { + const r = useReviewStore(s => s.activeReview)!; + const path = useReviewStore(s => s.path)!; + const mode = useReviewStore(s => s.mode); + const diff = useReviewStore(s => s.diff); + const fileContent = useReviewStore(s => s.fileContent); + const loading = useReviewStore(s => s.loadingDiff); + const busy = useReviewStore(s => s.busy); + const backToFiles = useReviewStore(s => s.backToFiles); + const openFullFile = useReviewStore(s => s.openFullFile); + const selectFile = useReviewStore(s => s.selectFile); + const addComment = useReviewStore(s => s.addComment); + const reply = useReviewStore(s => s.reply); + const resolve = useReviewStore(s => s.resolve); + + const threads = (r.threads || []).filter(t => t.file_path === path); + + return ( +
+
+ + {basename(path)} +
+ +
+
+
{path}
+
+ {loading &&
Loading…
} + {!loading && mode === 'file' && fileContent?.binary &&
Binary file — not shown.
} + {!loading && mode === 'file' && fileContent?.too_large &&
File too large to display.
} + {!loading && !(mode === 'file' && (fileContent?.binary || fileContent?.too_large)) && ( + + )} +
+
+ ); +} + +// ------------------------------------------------------------------ // +// Main panel // // ------------------------------------------------------------------ // export function FileChangesPanel() { - const modifiedFiles = useChatStore(s => s.modifiedFiles); const activeSession = useChatStore(s => s.activeSession); + const modifiedFiles = useChatStore(s => s.modifiedFiles); const fetchModifiedFiles = useChatStore(s => s.fetchModifiedFiles); - const [selectedFile, setSelectedFile] = useState(null); + + const setSession = useReviewStore(s => s.setSession); + const attached = useReviewStore(s => s.attached); + const activeReview = useReviewStore(s => s.activeReview); + const reviewPath = useReviewStore(s => s.path); + const selectReview = useReviewStore(s => s.selectReview); + const detach = useReviewStore(s => s.detach); + const error = useReviewStore(s => s.error); + const clearError = useReviewStore(s => s.clearError); + + const [selectedSnapshot, setSelectedSnapshot] = useState(null); + const [showAttach, setShowAttach] = useState(false); const [refreshing, setRefreshing] = useState(false); - // Reset selection when session changes - useEffect(() => { - setSelectedFile(null); - }, [activeSession]); + useEffect(() => { if (activeSession) setSession(activeSession); }, [activeSession, setSession]); + useEffect(() => { setSelectedSnapshot(null); }, [activeSession]); + + // --- Detail views take over the whole panel --- + if (selectedSnapshot) { + return setSelectedSnapshot(null)} />; + } + if (activeReview && reviewPath) return ; + if (activeReview) return ; + + // --- List view: session edits + attached worktrees --- + const totalAdd = modifiedFiles.reduce((s, f) => s + f.stats.additions, 0); + const totalDel = modifiedFiles.reduce((s, f) => s + f.stats.deletions, 0); const handleRefresh = async () => { setRefreshing(true); @@ -226,51 +373,64 @@ export function FileChangesPanel() { setRefreshing(false); }; - if (selectedFile) { - return ( - setSelectedFile(null)} - /> - ); - } - - const totalAdd = modifiedFiles.reduce((sum, f) => sum + f.stats.additions, 0); - const totalDel = modifiedFiles.reduce((sum, f) => sum + f.stats.deletions, 0); - return (
- {/* List header */} -
-
- {modifiedFiles.length} file{modifiedFiles.length !== 1 ? 's' : ''} - {totalAdd > 0 && +{totalAdd}} - {totalDel > 0 && −{totalDel}} + {error && ( +
+ {error} +
- -
+ )} - {/* File list */}
- {modifiedFiles.length === 0 ? ( -
- No files modified in this session + {/* Session edits (snapshot-based) */} +
+
+ {modifiedFiles.length} edit{modifiedFiles.length !== 1 ? 's' : ''} this session + {totalAdd > 0 && +{totalAdd}} + {totalDel > 0 && −{totalDel}} +
+ +
+ {modifiedFiles.map(file => ( + setSelectedSnapshot(file)} /> + ))} + + {/* Attached worktrees (git diffs + comments) */} +
+ Worktrees + +
+ {showAttach && setShowAttach(false)} />} + {attached.length === 0 && !showAttach && ( +
+ No worktrees attached. Use Attach to review a repo's changes here and leave line comments.
- ) : ( - modifiedFiles.map(file => ( - setSelectedFile(file)} - /> - )) )} + {attached.map(rev => ( +
+ + +
+ ))}
); diff --git a/web/src/components/Review/ReviewDiff.tsx b/web/src/components/Review/ReviewDiff.tsx new file mode 100644 index 00000000..4774fa18 --- /dev/null +++ b/web/src/components/Review/ReviewDiff.tsx @@ -0,0 +1,261 @@ +import { useMemo, useState } from 'react'; +import { MessageSquarePlus, Check, CornerDownRight, Bot, User } from 'lucide-react'; +import type { FileDiff } from '../../types/chat'; +import type { ReviewThread } from '../../types/review'; +import type { CommentAnchor } from '../../stores/reviewStore'; + +type RowType = 'addition' | 'deletion' | 'context' | 'info'; + +interface Row { + key: string; + kind: 'hunk' | 'line'; + type?: RowType; + oldLine?: number | null; + newLine?: number | null; + content: string; + header?: string; +} + +function buildRowsFromDiff(diff: FileDiff): Row[] { + const rows: Row[] = []; + diff.hunks.forEach((h, hi) => { + rows.push({ key: `h${hi}`, kind: 'hunk', content: '', header: h.header || `@@ -${h.old_start} +${h.new_start} @@` }); + h.lines.forEach((ln: any, li) => { + rows.push({ + key: `h${hi}l${li}`, + kind: 'line', + type: ln.type, + oldLine: ln.old_line ?? null, + newLine: ln.new_line ?? null, + content: ln.content ?? '', + }); + }); + }); + return rows; +} + +function buildRowsFromFile(content: string): Row[] { + return content.split('\n').map((line, i) => ({ + key: `f${i}`, + kind: 'line' as const, + type: 'context' as RowType, + newLine: i + 1, + content: line, + })); +} + +function anchorFor(row: Row, filePath: string): CommentAnchor { + const side: 'new' | 'old' = row.type === 'deletion' ? 'old' : 'new'; + const line = side === 'old' ? row.oldLine ?? null : row.newLine ?? null; + return { file_path: filePath, side, line_start: line, line_end: line, anchor_snippet: row.content }; +} + +const AUTHOR_STYLE: Record = { + human: 'text-hue-blue', + agent: 'text-hue-emerald', +}; + +function ThreadBlock({ + thread, busy, onReply, onResolve, +}: { + thread: ReviewThread; + busy: boolean; + onReply: (threadId: string, body: string) => void; + onResolve: (threadId: string) => void; +}) { + const [reply, setReply] = useState(''); + const resolved = thread.status === 'resolved'; + return ( +
+
+ thread on {thread.side === 'old' ? 'old' : 'new'} line {thread.line_start ?? '?'} + + {thread.status} + +
+
+ {(thread.comments || []).map(c => ( +
+
+ {c.author === 'agent' ? : } + {c.author} + {c.created_at?.slice(0, 16).replace('T', ' ')} +
+
{c.body}
+
+ ))} +
+ {!resolved && ( +
+