From 39aaf27b7d819cdf5a2bd482901824f2e25e61d6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 05:29:43 +0000 Subject: [PATCH 1/8] feat: add Review panel against the default-branch merge-base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the mock Diff tab with a singleton Review panel that lists the working tree versus merge-base(HEAD, default) — PR-style three-dot review — and previews each file with Pierre. On the default branch the base is HEAD, so only uncommitted work is shown. --- .../layout/content-panel/panels/README.md | 3 +- .../content-panel/panels/diff-panel.tsx | 70 ---- .../features/review/review-diff-adapter.tsx | 79 ++++ .../src/features/review/review-diff-pane.tsx | 119 ++++++ .../src/features/review/review-file-list.tsx | 99 +++++ .../review/review-file-status.test.ts | 17 + .../src/features/review/review-file-status.ts | 23 ++ apps/app/src/features/review/review-panel.tsx | 135 +++++++ apps/app/src/features/review/review-state.tsx | 44 ++ .../review/review-workspace-layout.tsx | 97 +++++ apps/app/src/features/review/use-git-diff.ts | 15 + .../app/src/features/review/use-git-review.ts | 15 + .../features/review/use-session-workspace.ts | 23 ++ apps/app/src/routes/__root.tsx | 4 +- packages/contract/package.json | 1 + packages/contract/src/git.ts | 116 ++++++ packages/contract/src/index.ts | 4 +- packages/server/src/errors.ts | 6 + packages/server/src/git/index.ts | 2 +- packages/server/src/git/name-status.ts | 47 +++ packages/server/src/git/service.ts | 382 +++++++++++++++++- packages/server/src/index.ts | 2 +- packages/server/src/rpc/context.ts | 2 + packages/server/src/rpc/git.ts | 85 ++++ packages/server/src/rpc/router.ts | 2 + packages/server/src/rpc/runtime.ts | 2 + packages/server/test/git.test.ts | 154 ++++++- packages/server/test/name-status.test.ts | 34 ++ packages/server/test/rpc-git.test.ts | 60 +++ packages/server/test/rpc-harness.ts | 2 + packages/server/test/rpc-session.test.ts | 2 + 31 files changed, 1539 insertions(+), 107 deletions(-) delete mode 100644 apps/app/src/components/layout/content-panel/panels/diff-panel.tsx create mode 100644 apps/app/src/features/review/review-diff-adapter.tsx create mode 100644 apps/app/src/features/review/review-diff-pane.tsx create mode 100644 apps/app/src/features/review/review-file-list.tsx create mode 100644 apps/app/src/features/review/review-file-status.test.ts create mode 100644 apps/app/src/features/review/review-file-status.ts create mode 100644 apps/app/src/features/review/review-panel.tsx create mode 100644 apps/app/src/features/review/review-state.tsx create mode 100644 apps/app/src/features/review/review-workspace-layout.tsx create mode 100644 apps/app/src/features/review/use-git-diff.ts create mode 100644 apps/app/src/features/review/use-git-review.ts create mode 100644 apps/app/src/features/review/use-session-workspace.ts create mode 100644 packages/contract/src/git.ts create mode 100644 packages/server/src/git/name-status.ts create mode 100644 packages/server/src/rpc/git.ts create mode 100644 packages/server/test/name-status.test.ts create mode 100644 packages/server/test/rpc-git.test.ts diff --git a/apps/app/src/components/layout/content-panel/panels/README.md b/apps/app/src/components/layout/content-panel/panels/README.md index e6c129b6e..edeb77a94 100644 --- a/apps/app/src/components/layout/content-panel/panels/README.md +++ b/apps/app/src/components/layout/content-panel/panels/README.md @@ -5,11 +5,12 @@ yet have real features behind them: | | arity | `create` | what it demonstrates | | ---------- | --------------------- | -------- | ----------------------------------------------------- | -| `diff` | singleton | — | the thin default handle; opening twice is one panel | | `browser` | family (`key: tabId`) | ✓ | an instance with its own store (url, loading) | | `terminal` | family (`key: id`) | ✓ | an instance whose store holds live, unpersisted state | The real Files entry panel and path-keyed file viewer live in `features/files/`. +The Review panel (branch change set vs the default base) lives in +`features/review/`. There is one tab strip and it belongs to the host. A panel that wants "several of a thing" — two shells, two files — opens several panels, so the strip stays diff --git a/apps/app/src/components/layout/content-panel/panels/diff-panel.tsx b/apps/app/src/components/layout/content-panel/panels/diff-panel.tsx deleted file mode 100644 index 620d99109..000000000 --- a/apps/app/src/components/layout/content-panel/panels/diff-panel.tsx +++ /dev/null @@ -1,70 +0,0 @@ -import { FileDiffIcon } from "lucide-react"; - -import { definePanel } from "../react/view"; - -// Placeholder content — see ./README.md. -const MOCK_FILES = [ - { path: "packages/server/src/harness/session.ts", added: 24, removed: 6 }, - { path: "apps/app/src/features/chat/chat.tsx", added: 8, removed: 8 }, - { path: "packages/contract/src/domain.ts", added: 3, removed: 0 }, -]; - -const MOCK_HUNK = [ - { sign: " ", text: " const session = yield* manager.sessionFor(ref)" }, - { sign: "-", text: " return session.snapshot()" }, - { sign: "+", text: " const snapshot = session.snapshot()" }, - { sign: "+", text: " yield* bus.publish(sessionUpdated(ref, snapshot))" }, - { sign: "+", text: " return snapshot" }, - { sign: " ", text: "}" }, -]; - -/** - * A singleton with no `create`: the default handle is everything it needs. The - * whole definition is data plus one render function. - */ -export const diffPanel = definePanel({ - type: "diff", - label: "Diff", - view: { - icon: FileDiffIcon, - render: () => , - }, -}); - -function DiffPanelView() { - return ( -
-
    - {MOCK_FILES.map((file) => ( -
  • - {file.path} - - +{file.added}{" "} - −{file.removed} - -
    • - ))} -
- 
-        {MOCK_HUNK.map((line) => (
-          

-            {line.sign}
-            {line.text}
-          

-        ))}
-
-
- ); -} diff --git a/apps/app/src/features/review/review-diff-adapter.tsx b/apps/app/src/features/review/review-diff-adapter.tsx new file mode 100644 index 000000000..3cbc6d442 --- /dev/null +++ b/apps/app/src/features/review/review-diff-adapter.tsx @@ -0,0 +1,79 @@ +import { parseDiffFromFile } from "@pierre/diffs"; +import { FileDiff, Virtualizer } from "@pierre/diffs/react"; +import { useMemo, useSyncExternalStore } from "react"; + +const DIFF_UNSAFE_CSS = ` + :host { + --diffs-font-family: var(--font-mono); + --diffs-light-bg: var(--background); + --diffs-dark-bg: var(--background); + --diffs-light: var(--foreground); + --diffs-dark: var(--foreground); + --diffs-fg-number-override: var(--muted-foreground); + --diffs-bg-buffer-override: var(--background); + --diffs-bg-context-override: var(--background); + --diffs-bg-context-gutter-override: var(--background); + --diffs-bg-separator-override: var(--border); + min-height: 100%; + width: 100%; + } +`; + +const getAppThemeType = (): "dark" | "light" => + document.documentElement.classList.contains("dark") ? "dark" : "light"; + +const subscribeToAppTheme = (listener: () => void): (() => void) => { + const observer = new MutationObserver(listener); + observer.observe(document.documentElement, { + attributeFilter: ["class"], + attributes: true, + }); + return () => observer.disconnect(); +}; + +export function ReviewDiffAdapter({ + path, + oldPath, + oldContents, + newContents, +}: { + path: string; + oldPath?: string; + oldContents: string | null; + newContents: string | null; +}) { + const themeType = useSyncExternalStore( + subscribeToAppTheme, + getAppThemeType, + () => "light" as const, + ); + const fileDiff = useMemo( + () => + parseDiffFromFile( + oldContents === null ? null : { name: oldPath ?? path, contents: oldContents }, + newContents === null ? null : { name: path, contents: newContents }, + ), + [newContents, oldContents, oldPath, path], + ); + const options = useMemo( + () => ({ + disableFileHeader: true, + overflow: "scroll" as const, + theme: { dark: "pierre-dark" as const, light: "pierre-light" as const }, + themeType, + unsafeCSS: DIFF_UNSAFE_CSS, + }), + [themeType], + ); + + return ( +
+ + + +
+ ); +} diff --git a/apps/app/src/features/review/review-diff-pane.tsx b/apps/app/src/features/review/review-diff-pane.tsx new file mode 100644 index 000000000..6ebaf5fb3 --- /dev/null +++ b/apps/app/src/features/review/review-diff-pane.tsx @@ -0,0 +1,119 @@ +import { ORPCError } from "@orpc/client"; +import type { UseQueryResult } from "@tanstack/react-query"; +import type { GitFileDiff } from "@vibest/contract/git"; +import { Button } from "@vibest/ui/components/button"; +import { Spinner } from "@vibest/ui/components/spinner"; +import { cn } from "@vibest/ui/lib/utils"; +import { FileDiffIcon, RefreshCwIcon } from "lucide-react"; +import { lazy, Suspense } from "react"; + +import { ReviewState } from "./review-state"; + +const ReviewDiffAdapter = lazy(() => + import("./review-diff-adapter").then((module) => ({ default: module.ReviewDiffAdapter })), +); + +export function ReviewDiffPane({ + diff, + path, + refreshing, + onRefresh, +}: { + diff: UseQueryResult; + path?: string; + refreshing: boolean; + onRefresh: () => void; +}) { + return ( +
+
+ + {path ?? "Select a file to review"} + + +
+ {path === undefined ? ( + + Select a file from the change set to open its diff against the review base. + + ) : diff.isPending ? ( +
+ +
+ ) : diff.isError ? ( + void diff.refetch()}> + {diffErrorMessage(diff.error)} + + ) : ( +
+ + +
+ } + > + + +
+ )} + + ); +} + +function diffErrorTitle(error: Error): string { + if (!(error instanceof ORPCError)) return "Unable to load diff"; + switch (error.code) { + case "NOT_FOUND": + return "File is no longer in the review"; + case "BINARY_FILE": + return "Binary preview unavailable"; + case "FILE_TOO_LARGE": + return "File too large to preview"; + default: + return "Unable to load diff"; + } +} + +function diffErrorMessage(error: Error): string { + if (!(error instanceof ORPCError)) return error.message; + switch (error.code) { + case "NOT_FOUND": + return "The file may have been committed, reverted, or renamed. Refresh the review."; + case "BINARY_FILE": + return "Binary preview unavailable."; + case "FILE_TOO_LARGE": { + const data = error.data as { size?: number; limit?: number } | undefined; + const size = data?.size; + const limit = data?.limit; + if (size !== undefined && limit !== undefined) { + return `${formatBytes(size)} exceeds the ${formatBytes(limit)} preview limit.`; + } + return "File too large to preview."; + } + case "PATH_ESCAPE": + return "This path resolves outside the project workspace."; + default: + return error.message; + } +} + +function formatBytes(bytes: number): string { + if (bytes < 1024) return `${bytes} B`; + const kibibytes = bytes / 1024; + if (kibibytes < 1024) return `${kibibytes.toFixed(1)} KiB`; + return `${(kibibytes / 1024).toFixed(1)} MiB`; +} diff --git a/apps/app/src/features/review/review-file-list.tsx b/apps/app/src/features/review/review-file-list.tsx new file mode 100644 index 000000000..50f27e426 --- /dev/null +++ b/apps/app/src/features/review/review-file-list.tsx @@ -0,0 +1,99 @@ +import type { GitReview, GitReviewFile, GitReviewFileStatus } from "@vibest/contract/git"; +import { Button } from "@vibest/ui/components/button"; +import { cn } from "@vibest/ui/lib/utils"; +import { RefreshCwIcon } from "lucide-react"; + +import { REVIEW_STATUS_BADGE, REVIEW_STATUS_LABEL, reviewHeading } from "./review-file-status"; + +const STATUS_CLASS: Record = { + modified: "text-amber-700 dark:text-amber-400", + added: "text-emerald-700 dark:text-emerald-400", + deleted: "text-rose-700 dark:text-rose-400", + renamed: "text-sky-700 dark:text-sky-400", + copied: "text-sky-700 dark:text-sky-400", +}; + +export function ReviewFileList({ + review, + selectedPath, + refreshing, + onSelect, + onRefresh, +}: { + review: GitReview; + selectedPath?: string; + refreshing: boolean; + onSelect: (path: string) => void; + onRefresh: () => void; +}) { + return ( +
+
+
+

+ {reviewHeading(review.branch, review.baseBranch)} +

+

+ {review.files.length === 1 ? "1 file" : `${review.files.length} files`} +

+
+ +
+
    + {review.files.map((file) => ( + + ))} +
+
+ ); +} + +function ReviewFileRow({ + file, + selected, + onSelect, +}: { + file: GitReviewFile; + selected: boolean; + onSelect: (path: string) => void; +}) { + const name = file.path.split("/").at(-1) || file.path; + return ( +
  • + +
    • + ); +} diff --git a/apps/app/src/features/review/review-file-status.test.ts b/apps/app/src/features/review/review-file-status.test.ts new file mode 100644 index 000000000..7eb75d155 --- /dev/null +++ b/apps/app/src/features/review/review-file-status.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; + +import { reviewHeading } from "./review-file-status"; + +describe("reviewHeading", () => { + it("names a feature-branch review against its base", () => { + expect(reviewHeading("feature/auth", "main")).toBe("feature/auth → main"); + }); + + it("names uncommitted work on the default branch", () => { + expect(reviewHeading("main", null)).toBe("Uncommitted changes on main"); + }); + + it("falls back when the branch name is missing", () => { + expect(reviewHeading(null, null)).toBe("Uncommitted changes"); + }); +}); diff --git a/apps/app/src/features/review/review-file-status.ts b/apps/app/src/features/review/review-file-status.ts new file mode 100644 index 000000000..e1b69fa85 --- /dev/null +++ b/apps/app/src/features/review/review-file-status.ts @@ -0,0 +1,23 @@ +import type { GitReviewFileStatus } from "@vibest/contract/git"; + +export const REVIEW_STATUS_LABEL: Record = { + modified: "Modified", + added: "Added", + deleted: "Deleted", + renamed: "Renamed", + copied: "Copied", +}; + +export const REVIEW_STATUS_BADGE: Record = { + modified: "M", + added: "A", + deleted: "D", + renamed: "R", + copied: "C", +}; + +export function reviewHeading(branch: string | null, baseBranch: string | null): string { + if (branch !== null && baseBranch !== null) return `${branch} → ${baseBranch}`; + if (branch !== null) return `Uncommitted changes on ${branch}`; + return "Uncommitted changes"; +} diff --git a/apps/app/src/features/review/review-panel.tsx b/apps/app/src/features/review/review-panel.tsx new file mode 100644 index 000000000..e03d7e638 --- /dev/null +++ b/apps/app/src/features/review/review-panel.tsx @@ -0,0 +1,135 @@ +import { ORPCError } from "@orpc/client"; +import { Spinner } from "@vibest/ui/components/spinner"; +import { GitCompareIcon } from "lucide-react"; +import { useCallback } from "react"; + +import { asRecord, type PanelHandle } from "@/components/layout/content-panel/core/panel"; +import { definePanel } from "@/components/layout/content-panel/react/view"; + +import { ReviewDiffPane } from "./review-diff-pane"; +import { ReviewFileList } from "./review-file-list"; +import { ReviewState } from "./review-state"; +import { ReviewWorkspaceLayout } from "./review-workspace-layout"; +import { useGitDiff } from "./use-git-diff"; +import { useGitReview } from "./use-git-review"; +import { useSessionWorkspace } from "./use-session-workspace"; + +export interface ReviewPayload { + readonly path?: string; +} + +export const reviewPanel = definePanel({ + type: "review", + label: "Review", + newPayload: () => ({}), + parse: (raw) => { + const record = asRecord(raw); + if (record === null) return {}; + return typeof record.path === "string" ? { path: record.path } : {}; + }, + view: { + icon: GitCompareIcon, + render: (instance) => , + }, +}); + +function ReviewPanelView({ instance }: { instance: PanelHandle }) { + const workspace = useSessionWorkspace(); + const cwd = workspace.data?.path; + const review = useGitReview(cwd); + const selectedPath = instance.payload.path; + const diff = useGitDiff(cwd, selectedPath); + const selectFile = useCallback((path: string) => instance.setPayload({ path }), [instance]); + + if (workspace.isPending) { + return ( +
    + +
    + ); + } + + if (workspace.isError) { + return ( + void workspace.refetch()}> + The project list could not be loaded. + + ); + } + + if (!workspace.data || cwd === undefined) { + return ( + + This session no longer resolves to an imported project. + + ); + } + + if (review.isPending) { + return ( +
    + +
    + ); + } + + if (review.isError) { + return ( + void review.refetch()}> + {reviewErrorMessage(review.error)} + + ); + } + + if (review.data.files.length === 0) { + return ( + + {review.data.baseBranch === null + ? "The working tree matches HEAD." + : `This branch has no changes against ${review.data.baseBranch}.`} + + ); + } + + const refreshing = review.isFetching || diff.isFetching; + const refresh = (): void => { + void Promise.all([review.refetch(), selectedPath === undefined ? undefined : diff.refetch()]); + }; + + return ( + + } + filesLabel={workspace.data.name} + preview={ + + } + /> + ); +} + +function reviewErrorTitle(error: Error): string { + if (error instanceof ORPCError && error.code === "NOT_REPOSITORY") { + return "Not a Git repository"; + } + return "Unable to load review"; +} + +function reviewErrorMessage(error: Error): string { + if (error instanceof ORPCError && error.code === "NOT_REPOSITORY") { + return "Open a Git project to review the branch against its default base."; + } + return error.message; +} diff --git a/apps/app/src/features/review/review-state.tsx b/apps/app/src/features/review/review-state.tsx new file mode 100644 index 000000000..fcbb61c6c --- /dev/null +++ b/apps/app/src/features/review/review-state.tsx @@ -0,0 +1,44 @@ +import { Button } from "@vibest/ui/components/button"; +import { + Empty, + EmptyContent, + EmptyDescription, + EmptyHeader, + EmptyMedia, + EmptyTitle, +} from "@vibest/ui/components/empty"; +import { GitCompareIcon, type LucideIcon } from "lucide-react"; +import type { ReactNode } from "react"; + +export function ReviewState({ + title, + children, + onRetry, + icon: Icon = GitCompareIcon, + prominentIcon = false, +}: { + title: string; + children: ReactNode; + onRetry?: () => void; + icon?: LucideIcon; + prominentIcon?: boolean; +}) { + return ( + + + + + + {title} + {children} + + {onRetry ? ( + + + + ) : null} + + ); +} diff --git a/apps/app/src/features/review/review-workspace-layout.tsx b/apps/app/src/features/review/review-workspace-layout.tsx new file mode 100644 index 000000000..8cb5ef3ed --- /dev/null +++ b/apps/app/src/features/review/review-workspace-layout.tsx @@ -0,0 +1,97 @@ +import { Button } from "@vibest/ui/components/button"; +import { Sheet, SheetHeader, SheetPopup, SheetTitle } from "@vibest/ui/components/sheet"; +import { useIsMobile } from "@vibest/ui/hooks/use-media-query"; +import { ListTreeIcon } from "lucide-react"; +import { type ReactNode, useId, useLayoutEffect, useRef, useState } from "react"; +import { Group, Panel, Separator } from "react-resizable-panels"; + +const MIN_SPLIT_WIDTH = 24 * 16 + 6; + +export function ReviewWorkspaceLayout({ + preview, + files, + filesLabel, +}: { + preview: ReactNode; + files: ReactNode; + filesLabel: string; +}) { + const isMobile = useIsMobile(); + const [isNarrow, setIsNarrow] = useState(false); + const [filesOpen, setFilesOpen] = useState(false); + const containerRef = useRef(null); + const drawerId = useId(); + + useLayoutEffect(() => { + const container = containerRef.current; + if (container === null) return; + + const updateWidth = (width: number): void => { + setIsNarrow(width < MIN_SPLIT_WIDTH); + }; + updateWidth(container.getBoundingClientRect().width); + + const observer = new ResizeObserver(([entry]) => { + if (entry !== undefined) updateWidth(entry.contentRect.width); + }); + observer.observe(container); + return () => observer.disconnect(); + }, []); + + const useDrawer = isMobile || isNarrow; + + return ( +
    + {useDrawer ? ( + <> + {preview} + + + + + Changed files + +
    {files}
    +     +     + + ) : ( + + + {preview} + + + + {files} + + + )} +
    + ); +} diff --git a/apps/app/src/features/review/use-git-diff.ts b/apps/app/src/features/review/use-git-diff.ts new file mode 100644 index 000000000..9ce90e8f3 --- /dev/null +++ b/apps/app/src/features/review/use-git-diff.ts @@ -0,0 +1,15 @@ +import { skipToken, useQuery } from "@tanstack/react-query"; +import { useRouteContext } from "@tanstack/react-router"; + +export function useGitDiff(cwd: string | undefined, path: string | undefined) { + const { orpcQueryUtils } = useRouteContext({ from: "__root__" }); + return useQuery({ + ...orpcQueryUtils.git.diff.queryOptions({ + input: cwd === undefined || path === undefined ? skipToken : { cwd, path }, + }), + refetchOnWindowFocus: "always", + staleTime: Infinity, + }); +} + +export type GitDiffQuery = ReturnType; diff --git a/apps/app/src/features/review/use-git-review.ts b/apps/app/src/features/review/use-git-review.ts new file mode 100644 index 000000000..638ea8188 --- /dev/null +++ b/apps/app/src/features/review/use-git-review.ts @@ -0,0 +1,15 @@ +import { skipToken, useQuery } from "@tanstack/react-query"; +import { useRouteContext } from "@tanstack/react-router"; + +export function useGitReview(cwd: string | undefined) { + const { orpcQueryUtils } = useRouteContext({ from: "__root__" }); + return useQuery({ + ...orpcQueryUtils.git.review.queryOptions({ + input: cwd === undefined ? skipToken : { cwd }, + }), + refetchOnWindowFocus: "always", + staleTime: Infinity, + }); +} + +export type GitReviewQuery = ReturnType; diff --git a/apps/app/src/features/review/use-session-workspace.ts b/apps/app/src/features/review/use-session-workspace.ts new file mode 100644 index 000000000..0afe86da8 --- /dev/null +++ b/apps/app/src/features/review/use-session-workspace.ts @@ -0,0 +1,23 @@ +import { useQuery, type UseQueryResult } from "@tanstack/react-query"; +import { useMatch, useRouteContext } from "@tanstack/react-router"; +import type { Project } from "@vibest/contract"; +import { useCallback } from "react"; + +/** Resolve the active session's project to its workspace path without coupling this feature to projects UI. */ +export function useSessionWorkspace(): UseQueryResult { + const projectId = useMatch({ + from: "/session/$sessionId", + shouldThrow: false, + select: (match) => match.loaderData?.projectId, + }); + const { orpcQueryUtils } = useRouteContext({ from: "__root__" }); + return useQuery({ + ...orpcQueryUtils.project.list.queryOptions(), + staleTime: Infinity, + // The select closes over projectId, so memoise it to preserve query result stability. + select: useCallback( + (projects: ReadonlyArray) => projects.find((project) => project.id === projectId), + [projectId], + ), + }); +} diff --git a/apps/app/src/routes/__root.tsx b/apps/app/src/routes/__root.tsx index 759b2d23e..fc2d7f842 100644 --- a/apps/app/src/routes/__root.tsx +++ b/apps/app/src/routes/__root.tsx @@ -17,7 +17,6 @@ import { import { AppSidebar } from "@/components/layout/app-sidebar"; import { CardPanel } from "@/components/layout/card-panel"; import { browserPanel } from "@/components/layout/content-panel/panels/browser-panel"; -import { diffPanel } from "@/components/layout/content-panel/panels/diff-panel"; import { terminalPanel } from "@/components/layout/content-panel/panels/terminal-panel"; import { ContentPanelSessionProvider } from "@/components/layout/content-panel/react/session-provider"; import { contentPanel } from "@/content-panel"; @@ -26,6 +25,7 @@ import { filesPanel } from "@/features/files/files-panel"; import { useProjectSessionTitle } from "@/features/projects/use-project-sessions"; import { useProject } from "@/features/projects/use-projects"; import { useSessionListSync } from "@/features/projects/use-session-list-sync"; +import { reviewPanel } from "@/features/review/review-panel"; import type { AppClients } from "@/lib/orpc"; import { sameSessionRef } from "@/lib/session-ref"; import { usePlatform } from "@/platform-context"; @@ -36,7 +36,7 @@ export interface RouterAppContext { queryClient: QueryClient; } -contentPanel.registerAll([filesPanel, filePanel, terminalPanel, diffPanel, browserPanel]); +contentPanel.registerAll([filesPanel, filePanel, reviewPanel, terminalPanel, browserPanel]); export const Route = createRootRouteWithContext()({ // Fetch the harness list once, right after the client connects and before diff --git a/packages/contract/package.json b/packages/contract/package.json index 7735b65c8..e9abee383 100644 --- a/packages/contract/package.json +++ b/packages/contract/package.json @@ -7,6 +7,7 @@ "exports": { ".": "./src/index.ts", "./fs": "./src/fs.ts", + "./git": "./src/git.ts", "./harness": "./src/harness.ts", "./project": "./src/project.ts", "./session": "./src/session.ts", diff --git a/packages/contract/src/git.ts b/packages/contract/src/git.ts new file mode 100644 index 000000000..7ae3a3c3c --- /dev/null +++ b/packages/contract/src/git.ts @@ -0,0 +1,116 @@ +import { oc } from "@orpc/contract"; +import { Schema } from "effect"; + +import { toStandardSchema } from "./domain"; + +const CwdInput = Schema.Struct({ cwd: Schema.String }); +const CwdPathInput = Schema.Struct({ + cwd: Schema.String, + path: Schema.String, +}); + +const pathData = toStandardSchema(Schema.Struct({ path: Schema.String })); +const pathEscapeData = toStandardSchema(Schema.Struct({ cwd: Schema.String, path: Schema.String })); +const cwdData = toStandardSchema(Schema.Struct({ cwd: Schema.String })); + +export const GitStatusFileSchema = Schema.Struct({ + path: Schema.String, + index: Schema.String, + worktree: Schema.String, + oldPath: Schema.optionalKey(Schema.String), +}); +export type GitStatusFile = typeof GitStatusFileSchema.Type; + +export const GitStatusSchema = Schema.Struct({ + branch: Schema.Union([Schema.String, Schema.Null]), + files: Schema.Array(GitStatusFileSchema), +}); +export type GitStatus = typeof GitStatusSchema.Type; + +export const GitBranchSchema = Schema.Struct({ + current: Schema.Union([Schema.String, Schema.Null]), + defaultBranch: Schema.Union([Schema.String, Schema.Null]), + branches: Schema.Array(Schema.String), +}); +export type GitBranch = typeof GitBranchSchema.Type; + +export const GitReviewFileStatusSchema = Schema.Literals([ + "modified", + "added", + "deleted", + "renamed", + "copied", +]); +export type GitReviewFileStatus = typeof GitReviewFileStatusSchema.Type; + +export const GitReviewFileSchema = Schema.Struct({ + path: Schema.String, + status: GitReviewFileStatusSchema, + oldPath: Schema.optionalKey(Schema.String), +}); +export type GitReviewFile = typeof GitReviewFileSchema.Type; + +/** + * A local review against the integration branch: three-dot + * `merge-base(default, HEAD)` plus the working tree. On the default branch + * (or with no default) the base is `HEAD`, so the set is uncommitted only. + */ +export const GitReviewSchema = Schema.Struct({ + branch: Schema.Union([Schema.String, Schema.Null]), + base: Schema.String, + baseBranch: Schema.Union([Schema.String, Schema.Null]), + files: Schema.Array(GitReviewFileSchema), +}); +export type GitReview = typeof GitReviewSchema.Type; + +export const GitFileDiffSchema = Schema.Struct({ + path: Schema.String, + status: GitReviewFileStatusSchema, + oldPath: Schema.optionalKey(Schema.String), + oldContents: Schema.Union([Schema.String, Schema.Null]), + newContents: Schema.Union([Schema.String, Schema.Null]), + binary: Schema.Boolean, +}); +export type GitFileDiff = typeof GitFileDiffSchema.Type; + +const cwdErrors = { + PATH_ESCAPE: { data: pathEscapeData }, + NOT_DIRECTORY: { data: pathData }, + NOT_REPOSITORY: { data: cwdData }, + GIT_FAILED: { data: cwdData }, +}; + +const diffErrors = { + ...cwdErrors, + NOT_FOUND: { data: pathData }, + BINARY_FILE: { data: pathData }, + FILE_TOO_LARGE: { + data: toStandardSchema( + Schema.Struct({ path: Schema.String, size: Schema.Number, limit: Schema.Number }), + ), + }, +}; + +/** + * Read-only git. Callers pass `cwd`; the server confines paths to that + * workspace and never writes. `review` / `diff` are the Code Review Panel + * surface — a change set vs the default branch, not staged/unstaged buckets. + */ +export const gitContract = { + status: oc + .input(toStandardSchema(CwdInput)) + .errors(cwdErrors) + .output(toStandardSchema(GitStatusSchema)), + branch: oc + .input(toStandardSchema(CwdInput)) + .errors(cwdErrors) + .output(toStandardSchema(GitBranchSchema)), + review: oc + .input(toStandardSchema(CwdInput)) + .errors(cwdErrors) + .output(toStandardSchema(GitReviewSchema)), + diff: oc + .input(toStandardSchema(CwdPathInput)) + .errors(diffErrors) + .output(toStandardSchema(GitFileDiffSchema)), +}; diff --git a/packages/contract/src/index.ts b/packages/contract/src/index.ts index f15d31ab1..869807821 100644 --- a/packages/contract/src/index.ts +++ b/packages/contract/src/index.ts @@ -1,4 +1,5 @@ import { fsContract } from "./fs"; +import { gitContract } from "./git"; import { harnessContract } from "./harness"; import { projectContract } from "./project"; import { sessionContract } from "./session"; @@ -11,7 +12,8 @@ export const contract = { session: sessionContract, project: projectContract, fs: fsContract, + git: gitContract, }; export type Contract = typeof contract; -export { fsContract, harnessContract, projectContract, sessionContract }; +export { fsContract, gitContract, harnessContract, projectContract, sessionContract }; diff --git a/packages/server/src/errors.ts b/packages/server/src/errors.ts index 48ab0282f..ada63e399 100644 --- a/packages/server/src/errors.ts +++ b/packages/server/src/errors.ts @@ -17,9 +17,15 @@ export class StoreWriteError extends Data.TaggedError("StoreWriteError")<{ }> {} export class GitError extends Data.TaggedError("GitError")<{ + readonly cwd: string; readonly cause: unknown; }> {} +/** `cwd` is not inside a Git work tree. */ +export class GitNotRepository extends Data.TaggedError("GitNotRepository")<{ + readonly cwd: string; +}> {} + export class SessionNotFound extends Data.TaggedError("SessionNotFound")<{ readonly projectId: string; readonly sessionId: string; diff --git a/packages/server/src/git/index.ts b/packages/server/src/git/index.ts index 850a40e56..bd565d869 100644 --- a/packages/server/src/git/index.ts +++ b/packages/server/src/git/index.ts @@ -1,2 +1,2 @@ -export type { BranchSummary, StatusResult } from "simple-git"; +export { parseNameStatus, parseNulPaths } from "./name-status"; export { GitService, GitServiceLayer } from "./service"; diff --git a/packages/server/src/git/name-status.ts b/packages/server/src/git/name-status.ts new file mode 100644 index 000000000..b008560d6 --- /dev/null +++ b/packages/server/src/git/name-status.ts @@ -0,0 +1,47 @@ +import type { GitReviewFile, GitReviewFileStatus } from "@vibest/contract/git"; + +const statusFromLetter = (letter: string): GitReviewFileStatus => { + if (letter === "A") return "added"; + if (letter === "D") return "deleted"; + if (letter === "R") return "renamed"; + if (letter === "C") return "copied"; + return "modified"; +}; + +/** + * Parse `git diff --name-status -z --find-renames` output. Rename/copy + * records are `STATUS\0old\0new\0`; everything else is `STATUS\0path\0`. + */ +export function parseNameStatus(raw: string): GitReviewFile[] { + if (raw === "") return []; + const parts = raw.split("\0"); + const files: GitReviewFile[] = []; + let index = 0; + while (index < parts.length) { + const code = parts[index]; + if (code === undefined || code === "") { + index += 1; + continue; + } + const letter = code[0] ?? ""; + if (letter === "R" || letter === "C") { + const oldPath = parts[index + 1]; + const nextPath = parts[index + 2]; + if (oldPath !== undefined && oldPath !== "" && nextPath !== undefined && nextPath !== "") { + files.push({ path: nextPath, status: statusFromLetter(letter), oldPath }); + } + index += 3; + continue; + } + const nextPath = parts[index + 1]; + if (nextPath !== undefined && nextPath !== "") { + files.push({ path: nextPath, status: statusFromLetter(letter) }); + } + index += 2; + } + return files; +} + +export function parseNulPaths(raw: string): string[] { + return raw.split("\0").filter((entry) => entry !== ""); +} diff --git a/packages/server/src/git/service.ts b/packages/server/src/git/service.ts index ece9153fa..9054db9ef 100644 --- a/packages/server/src/git/service.ts +++ b/packages/server/src/git/service.ts @@ -1,32 +1,370 @@ -import { Context, Effect, Layer } from "effect"; -import { type BranchSummary, simpleGit, type StatusResult } from "simple-git"; +import path from "node:path"; -import { GitError } from "../errors"; +import type { + GitBranch, + GitFileDiff, + GitReview, + GitReviewFile, + GitStatus, + GitStatusFile, +} from "@vibest/contract/git"; +import { Context, Effect, FileSystem, Layer } from "effect"; +import { simpleGit } from "simple-git"; + +import { + GitError, + GitNotRepository, + WorkspaceBinaryFile, + WorkspaceFileNotFound, + WorkspaceFileTooLarge, + WorkspaceNotDirectory, + WorkspaceNotFile, + WorkspacePathEscape, + WorkspaceReadError, +} from "../errors"; +import { FileSystemService } from "../fs"; +import { parseNameStatus, parseNulPaths } from "./name-status"; + +const MAX_FILE_BYTES = 2 * 1024 * 1024; +const NUL_BYTE = 0; +const BINARY_MAGIC_PREFIXES: ReadonlyArray> = [ + [0x25, 0x50, 0x44, 0x46, 0x2d], + [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a], + [0xff, 0xd8, 0xff], + [0x47, 0x49, 0x46, 0x38, 0x37, 0x61], + [0x47, 0x49, 0x46, 0x38, 0x39, 0x61], + [0x50, 0x4b, 0x03, 0x04], + [0x50, 0x4b, 0x05, 0x06], + [0x1f, 0x8b], + [0x7f, 0x45, 0x4c, 0x46], +]; +const DEFAULT_BRANCH_NAMES = ["main", "master", "trunk"] as const; + +const contains = (parent: string, child: string): boolean => { + const relative = path.relative(parent, child); + return ( + relative === "" || + (!path.isAbsolute(relative) && relative !== ".." && !relative.startsWith(`..${path.sep}`)) + ); +}; + +const toPosixPath = (value: string): string => value.split(path.sep).join("/"); + +const hasBinaryMagicPrefix = (bytes: Uint8Array): boolean => + BINARY_MAGIC_PREFIXES.some( + (prefix) => + bytes.byteLength >= prefix.length && prefix.every((byte, index) => bytes[index] === byte), + ); + +const isNotRepositoryMessage = (cause: unknown): boolean => { + const message = cause instanceof Error ? cause.message : String(cause); + return /not a git repository/i.test(message); +}; + +const decodeText = ( + bytes: Uint8Array, + relativePath: string, +): Effect.Effect => { + if (bytes.includes(NUL_BYTE) || hasBinaryMagicPrefix(bytes)) { + return Effect.fail(new WorkspaceBinaryFile({ path: relativePath })); + } + try { + return Effect.succeed(new TextDecoder("utf-8", { fatal: true }).decode(bytes)); + } catch { + return Effect.fail(new WorkspaceBinaryFile({ path: relativePath })); + } +}; + +type GitFailure = + | WorkspacePathEscape + | WorkspaceNotDirectory + | WorkspaceReadError + | GitNotRepository + | GitError; + +type GitDiffFailure = + | GitFailure + | WorkspaceFileNotFound + | WorkspaceNotFile + | WorkspaceBinaryFile + | WorkspaceFileTooLarge; /** - * `git` module — read-only, delegating to the `git` CLI via simple-git. Returns - * simple-git's own result types (`StatusResult`, `BranchSummary`) rather than - * re-modelling them. Only `status`/`branch` are exposed for now (design §4.5 / - * §8). + * Read-only `git` module. Workspace confinement matches `FileSystemService`: + * `cwd` must be an absolute directory, and every path git reports is rewritten + * relative to that directory (files outside it are dropped). */ export class GitService extends Context.Service< GitService, { - readonly status: (dir: string) => Effect.Effect; - readonly branch: (dir: string) => Effect.Effect; + readonly status: (cwd: string) => Effect.Effect; + readonly branch: (cwd: string) => Effect.Effect; + readonly review: (cwd: string) => Effect.Effect; + readonly diff: (cwd: string, path: string) => Effect.Effect; } >()("GitService") {} -export const GitServiceLayer: Layer.Layer = Layer.sync(GitService, () => ({ - status: (dir) => - Effect.tryPromise({ - try: () => simpleGit(dir).status(), - catch: (cause) => new GitError({ cause }), - }), - - branch: (dir) => - Effect.tryPromise({ - try: () => simpleGit(dir).branch(), - catch: (cause) => new GitError({ cause }), - }), -})); +export const GitServiceLayer: Layer.Layer< + GitService, + never, + FileSystem.FileSystem | FileSystemService +> = Layer.effect( + GitService, + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspace = yield* FileSystemService; + + const readError = (relativePath: string) => (cause: unknown) => + new WorkspaceReadError({ path: relativePath, cause }); + + const resolveRoot = (cwd: string) => + Effect.gen(function* () { + if (!path.isAbsolute(cwd)) { + return yield* new WorkspacePathEscape({ cwd, path: "." }); + } + const realRoot = yield* fs.realPath(cwd).pipe(Effect.mapError(readError("."))); + const info = yield* fs.stat(realRoot).pipe(Effect.mapError(readError("."))); + if (info.type !== "Directory") { + return yield* new WorkspaceNotDirectory({ path: "." }); + } + return realRoot; + }); + + const gitError = (cwd: string) => (cause: unknown) => + isNotRepositoryMessage(cause) ? new GitNotRepository({ cwd }) : new GitError({ cwd, cause }); + + const raw = (cwd: string, args: readonly string[]) => + Effect.tryPromise({ + try: () => simpleGit(cwd).raw([...args]), + catch: gitError(cwd), + }); + + const resolveRepoRoot = (cwd: string) => + raw(cwd, ["rev-parse", "--show-toplevel"]).pipe( + Effect.map((value) => value.trim()), + Effect.flatMap((toplevel) => + toplevel === "" + ? Effect.fail(new GitNotRepository({ cwd })) + : Effect.succeed(path.resolve(toplevel)), + ), + ); + + const toWorkspacePath = (cwd: string, repoRoot: string, gitPath: string): string | null => { + if (path.isAbsolute(gitPath) || gitPath.split(/[\\/]/).includes("..")) return null; + const absolute = path.resolve(repoRoot, gitPath); + if (!contains(cwd, absolute)) return null; + return toPosixPath(path.relative(cwd, absolute)) || gitPath; + }; + + const relocate = (cwd: string, repoRoot: string, file: GitReviewFile): GitReviewFile | null => { + const nextPath = toWorkspacePath(cwd, repoRoot, file.path); + if (nextPath === null) return null; + if (file.oldPath === undefined) return { ...file, path: nextPath }; + const oldPath = toWorkspacePath(cwd, repoRoot, file.oldPath); + if (oldPath === null) return { path: nextPath, status: file.status }; + return { path: nextPath, status: file.status, oldPath }; + }; + + const resolveDefaultRef = (cwd: string) => + Effect.gen(function* () { + const remoteHead = yield* raw(cwd, [ + "symbolic-ref", + "--quiet", + "refs/remotes/origin/HEAD", + ]).pipe( + Effect.map((value) => value.trim()), + Effect.catch(() => Effect.succeed("")), + ); + if (remoteHead.startsWith("refs/remotes/")) { + return remoteHead.slice("refs/remotes/".length); + } + const local = yield* raw(cwd, ["for-each-ref", "--format=%(refname:short)", "refs/heads"]); + const names = new Set( + local + .split("\n") + .map((name) => name.trim()) + .filter(Boolean), + ); + for (const name of DEFAULT_BRANCH_NAMES) { + if (names.has(name)) return name; + } + return null; + }); + + const shortBranchName = (ref: string): string => ref.replace(/^origin\//, ""); + + const isOnDefault = (current: string | null, defaultRef: string): boolean => { + if (current === null || current === "HEAD") return false; + return current === defaultRef || current === shortBranchName(defaultRef); + }; + + const resolveReviewBase = (cwd: string, current: string | null) => + Effect.gen(function* () { + const defaultRef = yield* resolveDefaultRef(cwd); + if (defaultRef === null || isOnDefault(current, defaultRef)) { + return { base: "HEAD", baseBranch: null as string | null }; + } + const mergeBase = yield* raw(cwd, ["merge-base", "HEAD", defaultRef]).pipe( + Effect.map((value) => value.trim()), + Effect.catch(() => Effect.succeed("")), + ); + return { + base: mergeBase === "" ? defaultRef : mergeBase, + baseBranch: shortBranchName(defaultRef), + }; + }); + + const currentBranch = (cwd: string) => + raw(cwd, ["rev-parse", "--abbrev-ref", "HEAD"]).pipe( + Effect.map((value) => { + const name = value.trim(); + return name === "" ? null : name; + }), + ); + + const reviewFiles = (cwd: string, repoRoot: string, base: string) => + Effect.gen(function* () { + const nameStatus = yield* raw(cwd, ["diff", "--name-status", "-z", "--find-renames", base]); + const tracked = parseNameStatus(nameStatus) + .map((file) => relocate(cwd, repoRoot, file)) + .filter((file): file is GitReviewFile => file !== null); + const untrackedRaw = yield* raw(cwd, ["ls-files", "-z", "--others", "--exclude-standard"]); + const seen = new Set(tracked.map((file) => file.path)); + const files = [...tracked]; + for (const gitPath of parseNulPaths(untrackedRaw)) { + const nextPath = toWorkspacePath(cwd, repoRoot, gitPath); + if (nextPath === null || seen.has(nextPath)) continue; + seen.add(nextPath); + files.push({ path: nextPath, status: "added" }); + } + files.sort((left, right) => + left.path.localeCompare(right.path, undefined, { numeric: true, sensitivity: "base" }), + ); + return files; + }); + + const readWorktreeText = (cwd: string, relativePath: string) => + workspace.readFileString(cwd, relativePath); + + const readBlobText = (cwd: string, base: string, blobPath: string) => + Effect.gen(function* () { + const sizeRaw = yield* raw(cwd, ["cat-file", "-s", `${base}:${blobPath}`]).pipe( + Effect.catch(() => Effect.succeed("")), + ); + if (sizeRaw.trim() === "") return null; + const size = Number(sizeRaw.trim()); + if (Number.isFinite(size) && size > MAX_FILE_BYTES) { + return yield* new WorkspaceFileTooLarge({ + path: blobPath, + size, + limit: MAX_FILE_BYTES, + }); + } + const text = yield* raw(cwd, ["cat-file", "-p", `${base}:${blobPath}`]); + const bytes = new TextEncoder().encode(text); + if (bytes.byteLength > MAX_FILE_BYTES) { + return yield* new WorkspaceFileTooLarge({ + path: blobPath, + size: bytes.byteLength, + limit: MAX_FILE_BYTES, + }); + } + return yield* decodeText(bytes, blobPath); + }); + + return { + status: (cwd) => + Effect.gen(function* () { + const realRoot = yield* resolveRoot(cwd); + const repoRoot = yield* resolveRepoRoot(realRoot); + const result = yield* Effect.tryPromise({ + try: () => simpleGit(realRoot).status(), + catch: gitError(realRoot), + }); + const files: GitStatusFile[] = []; + for (const file of result.files) { + const nextPath = toWorkspacePath(realRoot, repoRoot, file.path); + if (nextPath === null) continue; + const renameFrom = + "from" in file && typeof file.from === "string" ? file.from : undefined; + const relocatedFrom = + renameFrom === undefined + ? undefined + : toWorkspacePath(realRoot, repoRoot, renameFrom); + files.push({ + path: nextPath, + index: file.index, + worktree: file.working_dir, + ...(relocatedFrom === undefined || relocatedFrom === null + ? {} + : { oldPath: relocatedFrom }), + }); + } + return { branch: result.current ?? null, files }; + }), + + branch: (cwd) => + Effect.gen(function* () { + const realRoot = yield* resolveRoot(cwd); + const current = yield* currentBranch(realRoot); + const defaultRef = yield* resolveDefaultRef(realRoot); + const listed = yield* raw(realRoot, [ + "for-each-ref", + "--format=%(refname:short)", + "refs/heads", + ]); + const branches = listed + .split("\n") + .map((name) => name.trim()) + .filter(Boolean); + return { + current, + defaultBranch: defaultRef === null ? null : shortBranchName(defaultRef), + branches, + }; + }), + + review: (cwd) => + Effect.gen(function* () { + const realRoot = yield* resolveRoot(cwd); + const repoRoot = yield* resolveRepoRoot(realRoot); + const branch = yield* currentBranch(realRoot); + const { base, baseBranch } = yield* resolveReviewBase(realRoot, branch); + const files = yield* reviewFiles(realRoot, repoRoot, base); + return { branch, base, baseBranch, files }; + }), + + diff: (cwd, relativePath) => + Effect.gen(function* () { + const realRoot = yield* resolveRoot(cwd); + if (path.isAbsolute(relativePath) || relativePath.split(/[\\/]/).includes("..")) { + return yield* new WorkspacePathEscape({ cwd: realRoot, path: relativePath }); + } + const repoRoot = yield* resolveRepoRoot(realRoot); + const branch = yield* currentBranch(realRoot); + const { base } = yield* resolveReviewBase(realRoot, branch); + const files = yield* reviewFiles(realRoot, repoRoot, base); + const file = files.find((entry) => entry.path === relativePath); + if (file === undefined) { + return yield* new WorkspaceFileNotFound({ path: relativePath }); + } + const workspaceBlob = file.oldPath ?? file.path; + const blobPath = toPosixPath( + path.relative(repoRoot, path.resolve(realRoot, workspaceBlob)), + ); + const oldContents = + file.status === "added" ? null : yield* readBlobText(realRoot, base, blobPath); + const newContents = + file.status === "deleted" ? null : yield* readWorktreeText(realRoot, file.path); + return { + path: file.path, + status: file.status, + ...(file.oldPath === undefined ? {} : { oldPath: file.oldPath }), + oldContents, + newContents, + binary: false, + }; + }), + }; + }), +); diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index a30c31282..958b8796d 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -25,5 +25,5 @@ export const HarnessAgentDomainLayer = Layer.mergeAll( ProjectServiceLayer.pipe(Layer.provide(ProjectRepositoryLayer)), EventBusLayer, FileSystemServiceLayer, - GitServiceLayer, + GitServiceLayer.pipe(Layer.provide(FileSystemServiceLayer)), ); diff --git a/packages/server/src/rpc/context.ts b/packages/server/src/rpc/context.ts index 58878d428..8bcca291b 100644 --- a/packages/server/src/rpc/context.ts +++ b/packages/server/src/rpc/context.ts @@ -3,6 +3,7 @@ import type { FileSystem } from "effect/FileSystem"; import type { EventBus } from "../events"; import type { FileSystemService } from "../fs"; +import type { GitService } from "../git"; import type { HarnessAgentRegistry, HarnessAgentSessionService, @@ -21,4 +22,5 @@ export type RpcContext = WithEffectContext< | HarnessProbeService | ProjectService | FileSystemService + | GitService >; diff --git a/packages/server/src/rpc/git.ts b/packages/server/src/rpc/git.ts new file mode 100644 index 000000000..cdc706073 --- /dev/null +++ b/packages/server/src/rpc/git.ts @@ -0,0 +1,85 @@ +import "@orpc/experimental-effect/extensions/effect"; +import { implement } from "@orpc/server"; +import { gitContract } from "@vibest/contract/git"; +import { Effect } from "effect"; + +import { GitService } from "../git"; +import type { RpcContext } from "./context"; + +const orpc = implement(gitContract).$context(); + +export const gitRouter = orpc.router({ + status: orpc.status.effect(function* ({ input, errors }) { + const git = yield* GitService; + return yield* git.status(input.cwd).pipe( + Effect.catchTags({ + WorkspacePathEscape: (error) => + Effect.fail(errors.PATH_ESCAPE({ data: { cwd: error.cwd, path: error.path } })), + WorkspaceNotDirectory: (error) => + Effect.fail(errors.NOT_DIRECTORY({ data: { path: error.path } })), + WorkspaceReadError: () => Effect.fail(errors.GIT_FAILED({ data: { cwd: input.cwd } })), + GitNotRepository: (error) => + Effect.fail(errors.NOT_REPOSITORY({ data: { cwd: error.cwd } })), + GitError: (error) => Effect.fail(errors.GIT_FAILED({ data: { cwd: error.cwd } })), + }), + ); + }), + branch: orpc.branch.effect(function* ({ input, errors }) { + const git = yield* GitService; + return yield* git.branch(input.cwd).pipe( + Effect.catchTags({ + WorkspacePathEscape: (error) => + Effect.fail(errors.PATH_ESCAPE({ data: { cwd: error.cwd, path: error.path } })), + WorkspaceNotDirectory: (error) => + Effect.fail(errors.NOT_DIRECTORY({ data: { path: error.path } })), + WorkspaceReadError: () => Effect.fail(errors.GIT_FAILED({ data: { cwd: input.cwd } })), + GitNotRepository: (error) => + Effect.fail(errors.NOT_REPOSITORY({ data: { cwd: error.cwd } })), + GitError: (error) => Effect.fail(errors.GIT_FAILED({ data: { cwd: error.cwd } })), + }), + ); + }), + review: orpc.review.effect(function* ({ input, errors }) { + const git = yield* GitService; + return yield* git.review(input.cwd).pipe( + Effect.catchTags({ + WorkspacePathEscape: (error) => + Effect.fail(errors.PATH_ESCAPE({ data: { cwd: error.cwd, path: error.path } })), + WorkspaceNotDirectory: (error) => + Effect.fail(errors.NOT_DIRECTORY({ data: { path: error.path } })), + WorkspaceReadError: () => Effect.fail(errors.GIT_FAILED({ data: { cwd: input.cwd } })), + GitNotRepository: (error) => + Effect.fail(errors.NOT_REPOSITORY({ data: { cwd: error.cwd } })), + GitError: (error) => Effect.fail(errors.GIT_FAILED({ data: { cwd: error.cwd } })), + }), + ); + }), + diff: orpc.diff.effect(function* ({ input, errors }) { + const git = yield* GitService; + return yield* git.diff(input.cwd, input.path).pipe( + Effect.catchTags({ + WorkspacePathEscape: (error) => + Effect.fail(errors.PATH_ESCAPE({ data: { cwd: error.cwd, path: error.path } })), + WorkspaceNotDirectory: (error) => + Effect.fail(errors.NOT_DIRECTORY({ data: { path: error.path } })), + WorkspaceReadError: () => Effect.fail(errors.GIT_FAILED({ data: { cwd: input.cwd } })), + GitNotRepository: (error) => + Effect.fail(errors.NOT_REPOSITORY({ data: { cwd: error.cwd } })), + GitError: (error) => Effect.fail(errors.GIT_FAILED({ data: { cwd: error.cwd } })), + WorkspaceFileNotFound: (error) => + Effect.fail(errors.NOT_FOUND({ data: { path: error.path } })), + WorkspaceNotFile: (error) => Effect.fail(errors.NOT_FOUND({ data: { path: error.path } })), + WorkspaceBinaryFile: (error) => + Effect.fail(errors.BINARY_FILE({ data: { path: error.path } })), + WorkspaceFileTooLarge: (error) => + Effect.fail( + errors.FILE_TOO_LARGE({ + data: { path: error.path, size: error.size, limit: error.limit }, + }), + ), + }), + ); + }), +}); + +export type GitRouter = typeof gitRouter; diff --git a/packages/server/src/rpc/router.ts b/packages/server/src/rpc/router.ts index eefe3ddac..3dd393585 100644 --- a/packages/server/src/rpc/router.ts +++ b/packages/server/src/rpc/router.ts @@ -2,6 +2,7 @@ import { os } from "@orpc/server"; import type { RpcContext } from "./context"; import { fsRouter } from "./fs"; +import { gitRouter } from "./git"; import { harnessRouter } from "./harness"; import { projectRouter } from "./project"; import { sessionRouter } from "./session"; @@ -13,5 +14,6 @@ export const router = orpc.router({ session: sessionRouter, project: projectRouter, fs: fsRouter, + git: gitRouter, }); export type Router = typeof router; diff --git a/packages/server/src/rpc/runtime.ts b/packages/server/src/rpc/runtime.ts index 32bd45b66..a0987e526 100644 --- a/packages/server/src/rpc/runtime.ts +++ b/packages/server/src/rpc/runtime.ts @@ -8,6 +8,7 @@ import { Context, Effect, type FileSystem, Layer } from "effect"; import { PathsLayer } from "../config/paths"; import { EventBusLayer } from "../events"; import { FileSystemServiceLayer } from "../fs"; +import { GitServiceLayer } from "../git"; import { type HarnessAgentAdapter, HarnessAgentRegistry, @@ -142,6 +143,7 @@ export const AgentRuntimeLayer = Layer.mergeAll( HarnessListProvided, HarnessProbeProvided, FileSystemServiceLayer.pipe(Layer.provide(PlatformLayer)), + GitServiceLayer.pipe(Layer.provide(FileSystemServiceLayer), Layer.provide(PlatformLayer)), PlatformLayer, // For the HTTP request app: `HttpStaticServer` needs it to turn a file into a // response. Sealed by the vendor layer, hence no `Layer.provide` here. diff --git a/packages/server/test/git.test.ts b/packages/server/test/git.test.ts index 71543108b..48539c557 100644 --- a/packages/server/test/git.test.ts +++ b/packages/server/test/git.test.ts @@ -2,18 +2,21 @@ import assert from "node:assert/strict"; import path from "node:path"; import { layer } from "@effect/vitest"; -import { Effect, FileSystem } from "effect"; +import { Effect, FileSystem, Layer } from "effect"; import { simpleGit } from "simple-git"; -import { GitService, GitServiceLayer } from "../src/index"; +import { FileSystemServiceLayer } from "../src/fs"; +import { GitService, GitServiceLayer } from "../src/git"; import { NodePlatformLayer } from "./platform"; +const GitLayer = GitServiceLayer.pipe(Layer.provide(FileSystemServiceLayer)); + layer(NodePlatformLayer)("GitService", (it) => { /** A repo with one commit on `main`, removed when the test's scope closes. */ const repo = Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const dir = yield* fs.makeTempDirectoryScoped({ prefix: "vibest-git-" }); - yield* fs.writeFileString(path.join(dir, "a.txt"), "hi"); + yield* fs.writeFileString(path.join(dir, "a.txt"), "hi\n"); yield* Effect.promise(async () => { const git = simpleGit(dir); await git.raw(["init", "-b", "main"]); @@ -33,18 +36,151 @@ layer(NodePlatformLayer)("GitService", (it) => { const git = yield* GitService; const status = yield* git.status(dir); - assert.equal(status.current, "main"); - assert.ok(status.not_added.includes("untracked.txt")); - }).pipe(Effect.provide(GitServiceLayer)), + assert.equal(status.branch, "main"); + assert.ok(status.files.some((file) => file.path === "untracked.txt")); + }).pipe(Effect.provide(GitLayer)), ); - it.effect("lists branches", () => + it.effect("lists branches and the default branch", () => Effect.gen(function* () { const dir = yield* repo; const git = yield* GitService; const branch = yield* git.branch(dir); assert.equal(branch.current, "main"); - assert.ok(branch.all.includes("main")); - }).pipe(Effect.provide(GitServiceLayer)), + assert.equal(branch.defaultBranch, "main"); + assert.ok(branch.branches.includes("main")); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("reviews uncommitted work on the default branch against HEAD", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const dir = yield* repo; + yield* fs.writeFileString(path.join(dir, "a.txt"), "hello\n"); + yield* fs.writeFileString(path.join(dir, "added.txt"), "new\n"); + + const git = yield* GitService; + const review = yield* git.review(dir); + assert.equal(review.branch, "main"); + assert.equal(review.base, "HEAD"); + assert.equal(review.baseBranch, null); + assert.deepEqual(Array.from(review.files.map((file) => file.path)).toSorted(), [ + "a.txt", + "added.txt", + ]); + + const modified = yield* git.diff(dir, "a.txt"); + assert.equal(modified.status, "modified"); + assert.equal(modified.oldContents, "hi\n"); + assert.equal(modified.newContents, "hello\n"); + + const added = yield* git.diff(dir, "added.txt"); + assert.equal(added.status, "added"); + assert.equal(added.oldContents, null); + assert.equal(added.newContents, "new\n"); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("reviews a feature branch against merge-base with main", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const dir = yield* repo; + yield* Effect.promise(async () => { + const git = simpleGit(dir); + await git.checkoutLocalBranch("feature"); + }); + yield* fs.writeFileString(path.join(dir, "feature.txt"), "branch\n"); + yield* Effect.promise(async () => { + const git = simpleGit(dir); + await git.add("feature.txt"); + await git.commit("feature work"); + }); + + const git = yield* GitService; + const review = yield* git.review(dir); + assert.equal(review.branch, "feature"); + assert.equal(review.baseBranch, "main"); + assert.notEqual(review.base, "HEAD"); + assert.ok( + review.files.some((file) => file.path === "feature.txt" && file.status === "added"), + ); + + const diff = yield* git.diff(dir, "feature.txt"); + assert.equal(diff.oldContents, null); + assert.equal(diff.newContents, "branch\n"); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("ignores later main commits when reviewing a feature branch", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const dir = yield* repo; + yield* Effect.promise(async () => { + const git = simpleGit(dir); + await git.checkoutLocalBranch("feature"); + }); + yield* fs.writeFileString(path.join(dir, "feature.txt"), "branch\n"); + yield* Effect.promise(async () => { + const git = simpleGit(dir); + await git.add("feature.txt"); + await git.commit("feature work"); + await git.checkout("main"); + }); + yield* fs.writeFileString(path.join(dir, "a.txt"), "main-line\n"); + yield* fs.writeFileString(path.join(dir, "extra.txt"), "only on main\n"); + yield* Effect.promise(async () => { + const git = simpleGit(dir); + await git.add(["a.txt", "extra.txt"]); + await git.commit("main moved forward"); + await git.checkout("feature"); + }); + yield* fs.writeFileString(path.join(dir, "wip.txt"), "uncommitted\n"); + + const git = yield* GitService; + const review = yield* git.review(dir); + assert.equal(review.branch, "feature"); + assert.equal(review.baseBranch, "main"); + assert.deepEqual(Array.from(review.files.map((file) => file.path)).toSorted(), [ + "feature.txt", + "wip.txt", + ]); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("diffs a deleted file against the review base", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const dir = yield* repo; + yield* fs.remove(path.join(dir, "a.txt")); + + const git = yield* GitService; + const diff = yield* git.diff(dir, "a.txt"); + assert.equal(diff.status, "deleted"); + assert.equal(diff.oldContents, "hi\n"); + assert.equal(diff.newContents, null); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("rejects a relative cwd and a non-repository", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const dir = yield* fs.makeTempDirectoryScoped({ prefix: "vibest-not-git-" }); + const git = yield* GitService; + + const relative = yield* git.status("relative/workspace").pipe(Effect.flip); + assert.equal(relative._tag, "WorkspacePathEscape"); + + const missing = yield* git.review(dir).pipe(Effect.flip); + assert.equal(missing._tag, "GitNotRepository"); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("rejects a path that is not in the review set", () => + Effect.gen(function* () { + const dir = yield* repo; + const git = yield* GitService; + const missing = yield* git.diff(dir, "nope.ts").pipe(Effect.flip); + assert.equal(missing._tag, "WorkspaceFileNotFound"); + }).pipe(Effect.provide(GitLayer)), ); }); diff --git a/packages/server/test/name-status.test.ts b/packages/server/test/name-status.test.ts new file mode 100644 index 000000000..e3e2de22d --- /dev/null +++ b/packages/server/test/name-status.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; + +import { parseNameStatus, parseNulPaths } from "../src/git/name-status"; + +describe("parseNameStatus", () => { + it("parses modified, added, and deleted records", () => { + expect(parseNameStatus("M\0src/a.ts\0A\0src/b.ts\0D\0src/c.ts\0")).toEqual([ + { path: "src/a.ts", status: "modified" }, + { path: "src/b.ts", status: "added" }, + { path: "src/c.ts", status: "deleted" }, + ]); + }); + + it("parses rename and copy records with the old path", () => { + expect(parseNameStatus("R100\0old.ts\0new.ts\0C080\0src/a.ts\0src/a-copy.ts\0")).toEqual([ + { path: "new.ts", status: "renamed", oldPath: "old.ts" }, + { path: "src/a-copy.ts", status: "copied", oldPath: "src/a.ts" }, + ]); + }); + + it("treats type changes as modified", () => { + expect(parseNameStatus("T\0script\0")).toEqual([{ path: "script", status: "modified" }]); + }); + + it("returns an empty list for empty output", () => { + expect(parseNameStatus("")).toEqual([]); + }); +}); + +describe("parseNulPaths", () => { + it("splits untracked paths and drops empties", () => { + expect(parseNulPaths("notes.md\0tmp/a.ts\0")).toEqual(["notes.md", "tmp/a.ts"]); + }); +}); diff --git a/packages/server/test/rpc-git.test.ts b/packages/server/test/rpc-git.test.ts new file mode 100644 index 000000000..12f01637a --- /dev/null +++ b/packages/server/test/rpc-git.test.ts @@ -0,0 +1,60 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { simpleGit } from "simple-git"; +import { describe, expect, it } from "vitest"; + +import { makeRpcTestHarness } from "./rpc-harness"; + +async function makeRepo(): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-rpc-git-")); + fs.writeFileSync(path.join(dir, "a.txt"), "hi\n"); + const git = simpleGit(dir); + await git.raw(["init", "-b", "main"]); + await git.addConfig("user.email", "test@example.com"); + await git.addConfig("user.name", "Test"); + await git.add("."); + await git.commit("init"); + return dir; +} + +describe("git router", () => { + it("reviews uncommitted changes and returns a file diff", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-home-")); + const cwd = await makeRepo(); + fs.writeFileSync(path.join(cwd, "a.txt"), "hello\n"); + const harness = await makeRpcTestHarness(home); + try { + const review = await harness.client.git.review({ cwd }); + expect(review.branch).toBe("main"); + expect(review.base).toBe("HEAD"); + expect(review.baseBranch).toBeNull(); + expect(review.files).toEqual([{ path: "a.txt", status: "modified" }]); + + const diff = await harness.client.git.diff({ cwd, path: "a.txt" }); + expect(diff.oldContents).toBe("hi\n"); + expect(diff.newContents).toBe("hello\n"); + } finally { + await harness.dispose(); + } + }); + + it("maps a non-repository and a relative cwd to typed errors", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-home-")); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-not-git-")); + const harness = await makeRpcTestHarness(home); + try { + await expect(harness.client.git.review({ cwd: dir })).rejects.toMatchObject({ + code: "NOT_REPOSITORY", + data: { cwd: dir }, + }); + await expect(harness.client.git.status({ cwd: "relative/workspace" })).rejects.toMatchObject({ + code: "PATH_ESCAPE", + data: { cwd: "relative/workspace", path: "." }, + }); + } finally { + await harness.dispose(); + } + }); +}); diff --git a/packages/server/test/rpc-harness.ts b/packages/server/test/rpc-harness.ts index 5a38a5ba1..f8eb7b955 100644 --- a/packages/server/test/rpc-harness.ts +++ b/packages/server/test/rpc-harness.ts @@ -4,6 +4,7 @@ import { Layer, ManagedRuntime } from "effect"; import { layerPaths } from "../src/config/paths"; import { EventBusLayer } from "../src/events"; import { FileSystemServiceLayer } from "../src/fs"; +import { GitServiceLayer } from "../src/git"; import { HarnessAgentRegistry, HarnessAgentSessionManagerLayer, @@ -60,6 +61,7 @@ export async function makeRpcTestHarness( listLayer, probeLayer, FileSystemServiceLayer.pipe(Layer.provide(NodePlatformLayer)), + GitServiceLayer.pipe(Layer.provide(FileSystemServiceLayer), Layer.provide(NodePlatformLayer)), NodePlatformLayer, ), ); diff --git a/packages/server/test/rpc-session.test.ts b/packages/server/test/rpc-session.test.ts index 42953d0d2..dd2bca5ae 100644 --- a/packages/server/test/rpc-session.test.ts +++ b/packages/server/test/rpc-session.test.ts @@ -10,6 +10,7 @@ import { describe, expect, it } from "vitest"; import { layerPaths } from "../src/config/paths"; import { EventBusLayer } from "../src/events"; import { FileSystemServiceLayer } from "../src/fs"; +import { GitServiceLayer } from "../src/git"; import { HarnessAgentRegistry, HarnessAgentSessionManagerLayer, @@ -102,6 +103,7 @@ async function setup() { HarnessListLayer.pipe(Layer.provide(registryLayer), Layer.provide(NodeServices.layer)), HarnessProbeLayer.pipe(Layer.provide(registryLayer)), FileSystemServiceLayer.pipe(Layer.provide(NodeServices.layer)), + GitServiceLayer.pipe(Layer.provide(FileSystemServiceLayer), Layer.provide(NodeServices.layer)), NodeServices.layer, ); const runtime = ManagedRuntime.make(appLayer); From abba0486e274c043a0edc19aa9d18e29673ecbf0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 13:53:26 +0000 Subject: [PATCH 2/8] feat: add compare modes, Pierre file tree, and stacked review diffs Review now switches among uncommitted, committed three-dot, and vs-branch (including remote-tracking refs), shows the full workspace tree with git badges, and stacks collapsible Pierre diffs that the tree can locate. --- .../features/review/review-diff-adapter.tsx | 118 +++++++-- .../src/features/review/review-diff-pane.tsx | 129 ++++++---- .../src/features/review/review-file-list.tsx | 99 -------- .../review/review-file-status.test.ts | 73 +++++- .../src/features/review/review-file-status.ts | 84 ++++++- apps/app/src/features/review/review-panel.tsx | 163 +++++++++--- .../src/features/review/review-toolbar.tsx | 114 +++++++++ .../features/review/review-tree-adapter.tsx | 132 ++++++++++ .../src/features/review/review-tree-pane.tsx | 124 +++++++++ .../src/features/review/review-tree.test.ts | 86 +++++++ apps/app/src/features/review/review-tree.ts | 138 ++++++++++ .../review/review-workspace-layout.tsx | 24 +- .../{use-git-diff.ts => use-git-branch.ts} | 8 +- apps/app/src/features/review/use-git-diffs.ts | 31 +++ .../app/src/features/review/use-git-review.ts | 17 +- .../src/features/review/use-workspace-tree.ts | 15 ++ packages/contract/src/git.ts | 49 +++- packages/server/src/errors.ts | 5 + packages/server/src/git/service.ts | 236 ++++++++++++------ packages/server/src/rpc/git.ts | 6 +- packages/server/test/git.test.ts | 101 ++++++-- packages/server/test/rpc-git.test.ts | 18 ++ 22 files changed, 1419 insertions(+), 351 deletions(-) delete mode 100644 apps/app/src/features/review/review-file-list.tsx create mode 100644 apps/app/src/features/review/review-toolbar.tsx create mode 100644 apps/app/src/features/review/review-tree-adapter.tsx create mode 100644 apps/app/src/features/review/review-tree-pane.tsx create mode 100644 apps/app/src/features/review/review-tree.test.ts create mode 100644 apps/app/src/features/review/review-tree.ts rename apps/app/src/features/review/{use-git-diff.ts => use-git-branch.ts} (51%) create mode 100644 apps/app/src/features/review/use-git-diffs.ts create mode 100644 apps/app/src/features/review/use-workspace-tree.ts diff --git a/apps/app/src/features/review/review-diff-adapter.tsx b/apps/app/src/features/review/review-diff-adapter.tsx index 3cbc6d442..4be2b7108 100644 --- a/apps/app/src/features/review/review-diff-adapter.tsx +++ b/apps/app/src/features/review/review-diff-adapter.tsx @@ -1,6 +1,12 @@ import { parseDiffFromFile } from "@pierre/diffs"; -import { FileDiff, Virtualizer } from "@pierre/diffs/react"; -import { useMemo, useSyncExternalStore } from "react"; +import { + CodeView, + type CodeViewHandle, + type CodeViewItem, + type CodeViewReactOptions, +} from "@pierre/diffs/react"; +import type { GitFileDiff } from "@vibest/contract/git"; +import { useLayoutEffect, useMemo, useRef, useState, useSyncExternalStore } from "react"; const DIFF_UNSAFE_CSS = ` :host { @@ -17,6 +23,10 @@ const DIFF_UNSAFE_CSS = ` min-height: 100%; width: 100%; } + + [data-diffs-header] { + cursor: pointer; + } `; const getAppThemeType = (): "dark" | "light" => @@ -31,49 +41,103 @@ const subscribeToAppTheme = (listener: () => void): (() => void) => { return () => observer.disconnect(); }; +function parseReviewDiff(diff: GitFileDiff) { + return parseDiffFromFile( + diff.oldContents === null + ? null + : { name: diff.oldPath ?? diff.path, contents: diff.oldContents }, + diff.newContents === null ? null : { name: diff.path, contents: diff.newContents }, + ); +} + +function itemIdFromInstance(instance: object): string | undefined { + if (!("fileDiff" in instance)) return undefined; + const fileDiff = instance.fileDiff; + if (fileDiff === null || typeof fileDiff !== "object" || !("name" in fileDiff)) return undefined; + const name = fileDiff.name; + return typeof name === "string" ? name : undefined; +} + export function ReviewDiffAdapter({ - path, - oldPath, - oldContents, - newContents, + diffs, + locatePath, + locateRequest, }: { - path: string; - oldPath?: string; - oldContents: string | null; - newContents: string | null; + diffs: ReadonlyArray; + locatePath?: string; + locateRequest: number; }) { const themeType = useSyncExternalStore( subscribeToAppTheme, getAppThemeType, () => "light" as const, ); - const fileDiff = useMemo( + const codeViewRef = useRef>(null); + const [collapsed, setCollapsed] = useState>(() => new Set()); + const [appliedLocate, setAppliedLocate] = useState(locateRequest); + if (locateRequest !== appliedLocate) { + setAppliedLocate(locateRequest); + if (locatePath !== undefined && collapsed.has(locatePath)) { + const next = new Set(collapsed); + next.delete(locatePath); + setCollapsed(next); + } + } + const fileDiffs = useMemo( + () => diffs.map((diff) => ({ path: diff.path, fileDiff: parseReviewDiff(diff) })), + [diffs], + ); + + const items = useMemo>( () => - parseDiffFromFile( - oldContents === null ? null : { name: oldPath ?? path, contents: oldContents }, - newContents === null ? null : { name: path, contents: newContents }, - ), - [newContents, oldContents, oldPath, path], + fileDiffs.map(({ path, fileDiff }) => ({ + id: path, + type: "diff", + fileDiff, + collapsed: collapsed.has(path), + })), + [collapsed, fileDiffs], ); - const options = useMemo( + + const options = useMemo( () => ({ - disableFileHeader: true, - overflow: "scroll" as const, - theme: { dark: "pierre-dark" as const, light: "pierre-light" as const }, + overflow: "scroll", + stickyHeaders: true, + theme: { dark: "pierre-dark", light: "pierre-light" }, themeType, unsafeCSS: DIFF_UNSAFE_CSS, + onPostRender(node, instance, phase) { + if (phase === "unmount") return; + const header = node.shadowRoot?.querySelector("[data-diffs-header]"); + if (!(header instanceof HTMLElement)) return; + const id = itemIdFromInstance(instance); + if (id !== undefined) header.dataset.reviewPath = id; + if (header.dataset.reviewCollapseBound === "true") return; + header.dataset.reviewCollapseBound = "true"; + header.addEventListener("click", () => { + const path = header.dataset.reviewPath; + if (path === undefined) return; + setCollapsed((current) => { + const next = new Set(current); + if (next.has(path)) next.delete(path); + else next.add(path); + return next; + }); + }); + }, }), [themeType], ); + useLayoutEffect(() => { + if (locatePath === undefined) return; + if (!fileDiffs.some((entry) => entry.path === locatePath)) return; + codeViewRef.current?.scrollTo({ type: "item", id: locatePath, align: "start" }); + }, [fileDiffs, locatePath, locateRequest]); + return ( -
    - - - +
    +
    ); } diff --git a/apps/app/src/features/review/review-diff-pane.tsx b/apps/app/src/features/review/review-diff-pane.tsx index 6ebaf5fb3..6ac837807 100644 --- a/apps/app/src/features/review/review-diff-pane.tsx +++ b/apps/app/src/features/review/review-diff-pane.tsx @@ -1,79 +1,94 @@ import { ORPCError } from "@orpc/client"; -import type { UseQueryResult } from "@tanstack/react-query"; import type { GitFileDiff } from "@vibest/contract/git"; -import { Button } from "@vibest/ui/components/button"; import { Spinner } from "@vibest/ui/components/spinner"; -import { cn } from "@vibest/ui/lib/utils"; -import { FileDiffIcon, RefreshCwIcon } from "lucide-react"; +import { FileDiffIcon } from "lucide-react"; import { lazy, Suspense } from "react"; +import { emptyReviewMessage } from "./review-file-status"; import { ReviewState } from "./review-state"; +import type { GitDiffsQuery } from "./use-git-diffs"; +import type { GitReviewQuery } from "./use-git-review"; const ReviewDiffAdapter = lazy(() => import("./review-diff-adapter").then((module) => ({ default: module.ReviewDiffAdapter })), ); export function ReviewDiffPane({ - diff, + review, + diffs, path, - refreshing, - onRefresh, + locateRequest, }: { - diff: UseQueryResult; + review: GitReviewQuery; + diffs: GitDiffsQuery; path?: string; - refreshing: boolean; - onRefresh: () => void; + locateRequest: number; }) { - return ( -
    -
    - - {path ?? "Select a file to review"} - - + if (review.data !== undefined && review.data.files.length === 0) { + return ( + + {emptyReviewMessage(review.data)} + + ); + } + + if (diffs.some((diff) => diff.isPending)) { + return ( +
    +
    - {path === undefined ? ( - - Select a file from the change set to open its diff against the review base. - - ) : diff.isPending ? ( -
    - -
    - ) : diff.isError ? ( - void diff.refetch()}> - {diffErrorMessage(diff.error)} - - ) : ( -
    - - -
    - } - > - - -
    - )} + ); + } + + const loaded: GitFileDiff[] = []; + let firstError: Error | undefined; + for (const diff of diffs) { + if (diff.data !== undefined) { + loaded.push(diff.data); + continue; + } + if (diff.isError && diff.error !== null && !isSkippedDiffError(diff.error)) { + firstError ??= diff.error; + } + } + + if (loaded.length === 0 && firstError !== undefined) { + return ( + void review.refetch()}> + {diffErrorMessage(firstError)} + + ); + } + + if (loaded.length === 0) { + return ( + + Select a changed file in the tree to jump to its diff. + + ); + } + + return ( +
    + + +
    + } + > + +
    ); } +function isSkippedDiffError(error: Error): boolean { + return ( + error instanceof ORPCError && (error.code === "BINARY_FILE" || error.code === "FILE_TOO_LARGE") + ); +} + function diffErrorTitle(error: Error): string { if (!(error instanceof ORPCError)) return "Unable to load diff"; switch (error.code) { @@ -83,6 +98,8 @@ function diffErrorTitle(error: Error): string { return "Binary preview unavailable"; case "FILE_TOO_LARGE": return "File too large to preview"; + case "REF_NOT_FOUND": + return "Compare branch not found"; default: return "Unable to load diff"; } @@ -106,6 +123,8 @@ function diffErrorMessage(error: Error): string { } case "PATH_ESCAPE": return "This path resolves outside the project workspace."; + case "REF_NOT_FOUND": + return "Pick a local branch or a remote-tracking ref that already exists."; default: return error.message; } diff --git a/apps/app/src/features/review/review-file-list.tsx b/apps/app/src/features/review/review-file-list.tsx deleted file mode 100644 index 50f27e426..000000000 --- a/apps/app/src/features/review/review-file-list.tsx +++ /dev/null @@ -1,99 +0,0 @@ -import type { GitReview, GitReviewFile, GitReviewFileStatus } from "@vibest/contract/git"; -import { Button } from "@vibest/ui/components/button"; -import { cn } from "@vibest/ui/lib/utils"; -import { RefreshCwIcon } from "lucide-react"; - -import { REVIEW_STATUS_BADGE, REVIEW_STATUS_LABEL, reviewHeading } from "./review-file-status"; - -const STATUS_CLASS: Record = { - modified: "text-amber-700 dark:text-amber-400", - added: "text-emerald-700 dark:text-emerald-400", - deleted: "text-rose-700 dark:text-rose-400", - renamed: "text-sky-700 dark:text-sky-400", - copied: "text-sky-700 dark:text-sky-400", -}; - -export function ReviewFileList({ - review, - selectedPath, - refreshing, - onSelect, - onRefresh, -}: { - review: GitReview; - selectedPath?: string; - refreshing: boolean; - onSelect: (path: string) => void; - onRefresh: () => void; -}) { - return ( -
    -
    -
    -

    - {reviewHeading(review.branch, review.baseBranch)} -

    -

    - {review.files.length === 1 ? "1 file" : `${review.files.length} files`} -

    -
    - -
    -
      - {review.files.map((file) => ( - - ))} -
    -
    - ); -} - -function ReviewFileRow({ - file, - selected, - onSelect, -}: { - file: GitReviewFile; - selected: boolean; - onSelect: (path: string) => void; -}) { - const name = file.path.split("/").at(-1) || file.path; - return ( -
  • - -
    • - ); -} diff --git a/apps/app/src/features/review/review-file-status.test.ts b/apps/app/src/features/review/review-file-status.test.ts index 7eb75d155..e644bcb67 100644 --- a/apps/app/src/features/review/review-file-status.test.ts +++ b/apps/app/src/features/review/review-file-status.test.ts @@ -1,17 +1,78 @@ import { describe, expect, it } from "vitest"; -import { reviewHeading } from "./review-file-status"; +import { + emptyReviewMessage, + isReviewMode, + pierreGitStatus, + reviewHeading, + splitCompareRefs, +} from "./review-file-status"; describe("reviewHeading", () => { - it("names a feature-branch review against its base", () => { - expect(reviewHeading("feature/auth", "main")).toBe("feature/auth → main"); + it("names uncommitted work on the current branch", () => { + expect( + reviewHeading({ mode: "uncommitted", branch: "main", baseBranch: null, other: null }), + ).toBe("Uncommitted changes on main"); }); - it("names uncommitted work on the default branch", () => { - expect(reviewHeading("main", null)).toBe("Uncommitted changes on main"); + it("names a committed review against its base", () => { + expect( + reviewHeading({ + mode: "committed", + branch: "feature/auth", + baseBranch: "origin/main", + other: null, + }), + ).toBe("feature/auth → origin/main"); + }); + + it("names a branch comparison including remotes", () => { + expect( + reviewHeading({ + mode: "branch", + branch: "feature/auth", + baseBranch: "origin/main", + other: "origin/main", + }), + ).toBe("feature/auth → origin/main"); }); it("falls back when the branch name is missing", () => { - expect(reviewHeading(null, null)).toBe("Uncommitted changes"); + expect( + reviewHeading({ mode: "uncommitted", branch: null, baseBranch: null, other: null }), + ).toBe("Uncommitted changes"); + }); +}); + +describe("emptyReviewMessage", () => { + it("explains a clean working tree", () => { + expect(emptyReviewMessage({ mode: "uncommitted", baseBranch: null, other: null })).toBe( + "The working tree matches HEAD.", + ); + }); +}); + +describe("isReviewMode", () => { + it("accepts the three compare modes", () => { + expect(isReviewMode("uncommitted")).toBe(true); + expect(isReviewMode("committed")).toBe(true); + expect(isReviewMode("branch")).toBe(true); + expect(isReviewMode("pr")).toBe(false); + }); +}); + +describe("splitCompareRefs", () => { + it("keeps local and remote-tracking names in separate groups", () => { + expect(splitCompareRefs(["main", "feature", "origin/main", "origin/HEAD"])).toEqual({ + local: ["main", "feature"], + remote: ["origin/main", "origin/HEAD"], + }); + }); +}); + +describe("pierreGitStatus", () => { + it("maps copied to modified because Pierre has no copied badge", () => { + expect(pierreGitStatus("copied")).toBe("modified"); + expect(pierreGitStatus("renamed")).toBe("renamed"); }); }); diff --git a/apps/app/src/features/review/review-file-status.ts b/apps/app/src/features/review/review-file-status.ts index e1b69fa85..8d91eb2b5 100644 --- a/apps/app/src/features/review/review-file-status.ts +++ b/apps/app/src/features/review/review-file-status.ts @@ -1,4 +1,4 @@ -import type { GitReviewFileStatus } from "@vibest/contract/git"; +import type { GitReviewFile, GitReviewFileStatus, GitReviewMode } from "@vibest/contract/git"; export const REVIEW_STATUS_LABEL: Record = { modified: "Modified", @@ -16,8 +16,82 @@ export const REVIEW_STATUS_BADGE: Record = { copied: "C", }; -export function reviewHeading(branch: string | null, baseBranch: string | null): string { - if (branch !== null && baseBranch !== null) return `${branch} → ${baseBranch}`; - if (branch !== null) return `Uncommitted changes on ${branch}`; - return "Uncommitted changes"; +export const REVIEW_MODE_ITEMS = [ + { value: "uncommitted", label: "Uncommitted" }, + { value: "committed", label: "Committed" }, + { value: "branch", label: "vs branch" }, +] as const; + +export function isReviewMode(value: unknown): value is GitReviewMode { + return value === "uncommitted" || value === "committed" || value === "branch"; +} + +export function reviewHeading(review: { + mode: GitReviewMode; + branch: string | null; + baseBranch: string | null; + other: string | null; +}): string { + switch (review.mode) { + case "uncommitted": + return review.branch === null + ? "Uncommitted changes" + : `Uncommitted changes on ${review.branch}`; + case "committed": + if (review.branch !== null && review.baseBranch !== null) { + return `${review.branch} → ${review.baseBranch}`; + } + return "Committed changes"; + case "branch": { + const target = review.other ?? review.baseBranch; + if (review.branch !== null && target !== null) return `${review.branch} → ${target}`; + return "Branch comparison"; + } + } +} + +export function emptyReviewMessage(review: { + mode: GitReviewMode; + baseBranch: string | null; + other: string | null; +}): string { + switch (review.mode) { + case "uncommitted": + return "The working tree matches HEAD."; + case "committed": + return review.baseBranch === null + ? "HEAD matches the default branch." + : `This branch has no committed changes against ${review.baseBranch}.`; + case "branch": { + const target = review.other ?? review.baseBranch; + return target === null + ? "No changes against the selected branch." + : `No changes against ${target}.`; + } + } +} + +export function splitCompareRefs(branches: ReadonlyArray): { + local: string[]; + remote: string[]; +} { + const local: string[] = []; + const remote: string[] = []; + for (const name of branches) { + if (name.includes("/")) remote.push(name); + else local.push(name); + } + return { local, remote }; +} + +export function pierreGitStatus( + status: GitReviewFileStatus, +): "added" | "deleted" | "modified" | "renamed" { + return status === "copied" ? "modified" : status; +} + +export function reviewGitStatusEntries( + files: ReadonlyArray, +): ReadonlyArray<{ path: string; status: ReturnType }> { + return files.map((file) => ({ path: file.path, status: pierreGitStatus(file.status) })); } diff --git a/apps/app/src/features/review/review-panel.tsx b/apps/app/src/features/review/review-panel.tsx index e03d7e638..2cc66455e 100644 --- a/apps/app/src/features/review/review-panel.tsx +++ b/apps/app/src/features/review/review-panel.tsx @@ -1,20 +1,28 @@ import { ORPCError } from "@orpc/client"; +import type { GitReviewMode } from "@vibest/contract/git"; import { Spinner } from "@vibest/ui/components/spinner"; import { GitCompareIcon } from "lucide-react"; -import { useCallback } from "react"; +import { useCallback, useMemo, useState } from "react"; import { asRecord, type PanelHandle } from "@/components/layout/content-panel/core/panel"; +import { useContentPanel } from "@/components/layout/content-panel/react/hooks"; import { definePanel } from "@/components/layout/content-panel/react/view"; import { ReviewDiffPane } from "./review-diff-pane"; -import { ReviewFileList } from "./review-file-list"; +import { isReviewMode, reviewHeading } from "./review-file-status"; import { ReviewState } from "./review-state"; +import { ReviewToolbar } from "./review-toolbar"; +import { ReviewTreePane } from "./review-tree-pane"; import { ReviewWorkspaceLayout } from "./review-workspace-layout"; -import { useGitDiff } from "./use-git-diff"; +import { useGitBranch } from "./use-git-branch"; +import { useGitDiffs } from "./use-git-diffs"; import { useGitReview } from "./use-git-review"; import { useSessionWorkspace } from "./use-session-workspace"; +import { useWorkspaceTree } from "./use-workspace-tree"; export interface ReviewPayload { + readonly mode?: GitReviewMode; + readonly other?: string; readonly path?: string; } @@ -25,7 +33,14 @@ export const reviewPanel = definePanel({ parse: (raw) => { const record = asRecord(raw); if (record === null) return {}; - return typeof record.path === "string" ? { path: record.path } : {}; + const path = typeof record.path === "string" ? record.path : undefined; + const mode = isReviewMode(record.mode) ? record.mode : undefined; + const other = typeof record.other === "string" ? record.other : undefined; + return { + ...(path === undefined ? {} : { path }), + ...(mode === undefined ? {} : { mode }), + ...(other === undefined ? {} : { other }), + }; }, view: { icon: GitCompareIcon, @@ -35,11 +50,57 @@ export const reviewPanel = definePanel({ function ReviewPanelView({ instance }: { instance: PanelHandle }) { const workspace = useSessionWorkspace(); + const panel = useContentPanel(); const cwd = workspace.data?.path; - const review = useGitReview(cwd); + const mode = instance.payload.mode ?? "uncommitted"; + const branch = useGitBranch(cwd); + const other = + mode === "branch" + ? (instance.payload.other ?? branch.data?.defaultBranch ?? undefined) + : undefined; + const review = useGitReview(cwd, mode, other); + const tree = useWorkspaceTree(cwd); + const diffs = useGitDiffs(cwd, review.data?.files ?? [], mode, other); + const [locateRequest, setLocateRequest] = useState(0); const selectedPath = instance.payload.path; - const diff = useGitDiff(cwd, selectedPath); - const selectFile = useCallback((path: string) => instance.setPayload({ path }), [instance]); + + const selectFile = useCallback( + (path: string) => { + instance.setPayload((current) => ({ ...current, path })); + setLocateRequest((current) => current + 1); + }, + [instance], + ); + + const setMode = useCallback( + (next: GitReviewMode) => { + instance.setPayload((current) => { + if (next === "branch") { + const nextOther = current.other ?? branch.data?.defaultBranch ?? undefined; + return { + ...current, + mode: next, + ...(nextOther === undefined ? {} : { other: nextOther }), + }; + } + const { other: _other, ...rest } = current; + return { ...rest, mode: next }; + }); + }, + [branch.data?.defaultBranch, instance], + ); + + const setOther = useCallback( + (next: string) => { + instance.setPayload((current) => ({ ...current, mode: "branch", other: next })); + }, + [instance], + ); + + const heading = useMemo( + () => (review.data === undefined ? "" : reviewHeading(review.data)), + [review.data], + ); if (workspace.isPending) { return ( @@ -57,7 +118,7 @@ function ReviewPanelView({ instance }: { instance: PanelHandle }) ); } - if (!workspace.data || cwd === undefined) { + if (!workspace.data || cwd === undefined || panel === null) { return ( This session no longer resolves to an imported project. @@ -65,7 +126,18 @@ function ReviewPanelView({ instance }: { instance: PanelHandle }) ); } - if (review.isPending) { + if (mode === "branch" && other === undefined && !branch.isPending) { + return ( + + This repository has no local default branch or remote-tracking ref to compare against. + + ); + } + + if ( + (review.isPending && review.data === undefined) || + (mode === "branch" && other === undefined) + ) { return (
    @@ -73,7 +145,7 @@ function ReviewPanelView({ instance }: { instance: PanelHandle }) ); } - if (review.isError) { + if (review.isError && review.data === undefined) { return ( void review.refetch()}> {reviewErrorMessage(review.error)} @@ -81,38 +153,47 @@ function ReviewPanelView({ instance }: { instance: PanelHandle }) ); } - if (review.data.files.length === 0) { - return ( - - {review.data.baseBranch === null - ? "The working tree matches HEAD." - : `This branch has no changes against ${review.data.baseBranch}.`} - - ); - } - - const refreshing = review.isFetching || diff.isFetching; + const refreshing = review.isFetching || branch.isFetching || tree.isFetching; const refresh = (): void => { - void Promise.all([review.refetch(), selectedPath === undefined ? undefined : diff.refetch()]); + void Promise.all([ + review.refetch(), + branch.refetch(), + tree.refetch(), + ...diffs.map((diff) => diff.refetch()), + ]); }; return ( } filesLabel={workspace.data.name} preview={ + } + toolbar={ + } @@ -121,15 +202,25 @@ function ReviewPanelView({ instance }: { instance: PanelHandle }) } function reviewErrorTitle(error: Error): string { - if (error instanceof ORPCError && error.code === "NOT_REPOSITORY") { - return "Not a Git repository"; + if (!(error instanceof ORPCError)) return "Unable to load review"; + switch (error.code) { + case "NOT_REPOSITORY": + return "Not a Git repository"; + case "REF_NOT_FOUND": + return "Compare branch not found"; + default: + return "Unable to load review"; } - return "Unable to load review"; } function reviewErrorMessage(error: Error): string { - if (error instanceof ORPCError && error.code === "NOT_REPOSITORY") { - return "Open a Git project to review the branch against its default base."; + if (!(error instanceof ORPCError)) return error.message; + switch (error.code) { + case "NOT_REPOSITORY": + return "Open a Git project to review uncommitted work, commits, or another branch."; + case "REF_NOT_FOUND": + return "Pick a local branch or a remote-tracking ref that already exists."; + default: + return error.message; } - return error.message; } diff --git a/apps/app/src/features/review/review-toolbar.tsx b/apps/app/src/features/review/review-toolbar.tsx new file mode 100644 index 000000000..c26615304 --- /dev/null +++ b/apps/app/src/features/review/review-toolbar.tsx @@ -0,0 +1,114 @@ +import type { GitBranch, GitReviewMode } from "@vibest/contract/git"; +import { Button } from "@vibest/ui/components/button"; +import { + Select, + SelectContent, + SelectGroup, + SelectGroupLabel, + SelectItem, + SelectTrigger, + SelectValue, +} from "@vibest/ui/components/select"; +import { cn } from "@vibest/ui/lib/utils"; +import { RefreshCwIcon } from "lucide-react"; + +import { REVIEW_MODE_ITEMS, splitCompareRefs } from "./review-file-status"; + +export function ReviewToolbar({ + mode, + other, + branch, + heading, + refreshing, + onModeChange, + onOtherChange, + onRefresh, +}: { + mode: GitReviewMode; + other: string | undefined; + branch: GitBranch | undefined; + heading: string; + refreshing: boolean; + onModeChange: (mode: GitReviewMode) => void; + onOtherChange: (other: string) => void; + onRefresh: () => void; +}) { + const refs = splitCompareRefs(branch?.branches ?? []); + const otherValue = other ?? branch?.defaultBranch ?? null; + const otherItems = (branch?.branches ?? []).map((name) => ({ label: name, value: name })); + + return ( +
    + + {mode === "branch" ? ( + + ) : ( +

    + {heading} +

    + )} + +
    + ); +} diff --git a/apps/app/src/features/review/review-tree-adapter.tsx b/apps/app/src/features/review/review-tree-adapter.tsx new file mode 100644 index 000000000..0a61b54e8 --- /dev/null +++ b/apps/app/src/features/review/review-tree-adapter.tsx @@ -0,0 +1,132 @@ +import { FileTree as PierreFileTree } from "@pierre/trees/react"; +import type { WorkspaceTreeEntry } from "@vibest/contract/fs"; +import { + type CSSProperties, + type KeyboardEvent, + type MouseEvent, + useEffect, + useLayoutEffect, + useMemo, + useRef, +} from "react"; + +import { + getReviewFileTree, + isOpenableTreeEntry, + symlinkDescription, + syncReviewFileTree, +} from "./review-tree"; + +const TREE_STYLE = { + height: "100%", + width: "100%", + "--trees-bg-override": "var(--background)", + "--trees-bg-muted-override": "var(--muted)", + "--trees-border-color-override": "var(--border)", + "--trees-fg-override": "var(--foreground)", + "--trees-fg-muted-override": "var(--muted-foreground)", + "--trees-focus-ring-color-override": "var(--ring)", + "--trees-font-family-override": "var(--font-mono)", + "--trees-font-size-override": "12px", + "--trees-selected-bg-override": "var(--accent)", + "--trees-selected-fg-override": "var(--accent-foreground)", +} as CSSProperties; + +function pathFromComposedEvent(event: MouseEvent): string | null { + for (const target of event.nativeEvent.composedPath()) { + if (target instanceof HTMLElement && target.dataset.itemPath !== undefined) { + return target.dataset.itemPath; + } + } + return null; +} + +export function ReviewTreeAdapter({ + sessionId, + entries, + gitStatus, + onSelectFile, +}: { + sessionId: string; + entries: ReadonlyArray; + gitStatus: ReadonlyArray<{ + path: string; + status: "added" | "deleted" | "modified" | "renamed"; + }>; + onSelectFile: (path: string) => void; +}) { + const state = useMemo(() => getReviewFileTree(sessionId), [sessionId]); + const containerRef = useRef(null); + + useLayoutEffect(() => { + syncReviewFileTree(state, entries, gitStatus); + }, [entries, gitStatus, state]); + + useEffect(() => { + const host = containerRef.current?.querySelector("file-tree-container"); + const shadowRoot = host?.shadowRoot; + if (shadowRoot === undefined || shadowRoot === null) return; + + const annotateRows = (): void => { + for (const row of shadowRoot.querySelectorAll("[data-item-path]")) { + const path = row.dataset.itemPath; + const entry = path === undefined ? undefined : state.entryByPath.get(path); + const description = entry === undefined ? null : symlinkDescription(entry); + if (description === null) { + row.removeAttribute("aria-description"); + row.removeAttribute("aria-disabled"); + continue; + } + row.setAttribute("aria-description", description); + if (isOpenableTreeEntry(entry)) row.removeAttribute("aria-disabled"); + else row.setAttribute("aria-disabled", "true"); + } + }; + + annotateRows(); + const observer = new MutationObserver(annotateRows); + observer.observe(shadowRoot, { + attributeFilter: ["data-item-path"], + attributes: true, + childList: true, + subtree: true, + }); + return () => observer.disconnect(); + }, [entries, state]); + + const openPath = (path: string | null): void => { + if (path === null) return; + const entry = state.entryByPath.get(path); + if (!isOpenableTreeEntry(entry)) return; + state.model.getItem(path)?.select(); + onSelectFile(path); + }; + + const handleClick = (event: MouseEvent): void => { + openPath(pathFromComposedEvent(event)); + }; + + const handleKeyDown = (event: KeyboardEvent): void => { + if (event.key !== "Enter" || event.defaultPrevented) return; + const focusedPath = state.model.getFocusedPath(); + if ( + !isOpenableTreeEntry(focusedPath === null ? undefined : state.entryByPath.get(focusedPath)) + ) { + return; + } + event.preventDefault(); + openPath(focusedPath); + }; + + return ( +
    + +
    + ); +} diff --git a/apps/app/src/features/review/review-tree-pane.tsx b/apps/app/src/features/review/review-tree-pane.tsx new file mode 100644 index 000000000..7b172a986 --- /dev/null +++ b/apps/app/src/features/review/review-tree-pane.tsx @@ -0,0 +1,124 @@ +import { ORPCError } from "@orpc/client"; +import type { GitReviewFile } from "@vibest/contract/git"; +import { + Empty, + EmptyContent, + EmptyDescription, + EmptyMedia, + EmptyTitle, +} from "@vibest/ui/components/empty"; +import { Spinner } from "@vibest/ui/components/spinner"; +import { FilesIcon, TriangleAlertIcon } from "lucide-react"; +import { lazy, Suspense, useMemo } from "react"; + +import { reviewGitStatusEntries } from "./review-file-status"; +import { unionDeletedReviewEntries } from "./review-tree"; +import type { WorkspaceTreeQuery } from "./use-workspace-tree"; + +const ReviewTreeAdapter = lazy(() => + import("./review-tree-adapter").then((module) => ({ default: module.ReviewTreeAdapter })), +); + +export function ReviewTreePane({ + sessionId, + workspaceName, + workspacePath, + tree, + files, + onSelectFile, +}: { + sessionId: string; + workspaceName: string; + workspacePath: string; + tree: WorkspaceTreeQuery; + files: ReadonlyArray; + onSelectFile: (path: string) => void; +}) { + const entries = useMemo( + () => (tree.data === undefined ? [] : unionDeletedReviewEntries(tree.data.entries, files)), + [files, tree.data], + ); + const gitStatus = useMemo(() => reviewGitStatusEntries(files), [files]); + + return ( +
    +
    + + {workspaceName} + +
    + + {tree.isError && tree.data !== undefined ? ( +
    + + + Refresh failed; showing the previous file tree. + +
    + ) : null} + + {tree.isPending ? ( +
    + +
    + ) : tree.data === undefined ? ( + + + + + +
    + Unable to load files + {treeErrorMessage(tree.error)} +
    +     +     + ) : entries.length === 0 ? ( + + + + + +
    + No files + This workspace contains no visible files. +
    +     +     + ) : ( + + +
    + } + > + + + )} +
    + ); +} + +function treeErrorMessage(error: Error | null): string { + if (error === null) return "The workspace file tree could not be loaded."; + if (!(error instanceof ORPCError)) return error.message; + switch (error.code) { + case "NOT_DIRECTORY": + return "The project workspace is no longer a directory."; + case "PATH_ESCAPE": + return "The project workspace path is invalid."; + case "READ_FAILED": + return "The workspace may have moved, been deleted, or become unreadable."; + default: + return error.message; + } +} diff --git a/apps/app/src/features/review/review-tree.test.ts b/apps/app/src/features/review/review-tree.test.ts new file mode 100644 index 000000000..bb8be48a0 --- /dev/null +++ b/apps/app/src/features/review/review-tree.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from "vitest"; + +import { + getReviewFileTree, + isOpenableTreeEntry, + symlinkDescription, + syncReviewFileTree, + toPierrePath, + unionDeletedReviewEntries, +} from "./review-tree"; + +describe("review file tree", () => { + it("converts directory paths to Pierre directory identifiers", () => { + expect(toPierrePath({ path: "src", type: "directory" })).toBe("src/"); + expect(toPierrePath({ path: "src/index.ts", type: "file" })).toBe("src/index.ts"); + }); + + it("adds deleted review paths that are gone from the workspace tree", () => { + const entries = unionDeletedReviewEntries( + [ + { path: "src", type: "directory" }, + { path: "src/keep.ts", type: "file" }, + ], + [ + { path: "src/gone.ts", status: "deleted" }, + { path: "legacy/old.ts", status: "deleted" }, + { path: "src/keep.ts", status: "modified" }, + ], + ); + expect(entries).toEqual([ + { path: "src", type: "directory" }, + { path: "src/keep.ts", type: "file" }, + { path: "src/gone.ts", type: "file" }, + { path: "legacy", type: "directory" }, + { path: "legacy/old.ts", type: "file" }, + ]); + }); + + it("preserves expanded directories across complete tree resets", () => { + const state = getReviewFileTree(`test-${crypto.randomUUID()}`); + syncReviewFileTree( + state, + [ + { path: "src", type: "directory" }, + { path: "src/index.ts", type: "file" }, + ], + [], + ); + + const src = state.model.getItem("src/"); + expect(src?.isDirectory()).toBe(true); + if (src === null || !src.isDirectory() || !("expand" in src)) { + throw new Error("src directory missing"); + } + src.expand(); + + syncReviewFileTree( + state, + [ + { path: "README.md", type: "file" }, + { path: "src", type: "directory" }, + { path: "src/index.ts", type: "file" }, + { path: "src/new.ts", type: "file" }, + ], + [{ path: "src/new.ts", status: "added" }], + ); + + const refreshedSrc = state.model.getItem("src/"); + expect(refreshedSrc?.isDirectory()).toBe(true); + if (refreshedSrc === null || !refreshedSrc.isDirectory() || !("isExpanded" in refreshedSrc)) { + throw new Error("refreshed src directory missing"); + } + expect(refreshedSrc.isExpanded()).toBe(true); + state.model.cleanUp(); + }); + + it("describes why non-file symlinks cannot be opened", () => { + expect( + symlinkDescription({ path: "dir-link", type: "symlink", symlinkTarget: "directory" }), + ).toContain("disabled"); + }); + + it("treats regular files as openable", () => { + expect(isOpenableTreeEntry({ path: "a.ts", type: "file" })).toBe(true); + }); +}); diff --git a/apps/app/src/features/review/review-tree.ts b/apps/app/src/features/review/review-tree.ts new file mode 100644 index 000000000..f9368e1db --- /dev/null +++ b/apps/app/src/features/review/review-tree.ts @@ -0,0 +1,138 @@ +import { FileTree, prepareFileTreeInput } from "@pierre/trees"; +import type { WorkspaceTreeEntry } from "@vibest/contract/fs"; +import type { GitReviewFile } from "@vibest/contract/git"; + +export interface ReviewFileTree { + readonly model: FileTree; + entries: ReadonlyArray; + entryByPath: ReadonlyMap; + directoryPaths: ReadonlySet; +} + +const reviewTrees = new Map(); + +export function toPierrePath(entry: WorkspaceTreeEntry): string { + return entry.type === "directory" ? `${entry.path}/` : entry.path; +} + +export function symlinkDescription(entry: WorkspaceTreeEntry): string | null { + if (entry.type !== "symlink") return null; + switch (entry.symlinkTarget) { + case "file": + return "Symbolic link to a file"; + case "directory": + return "Symbolic link to a directory; opening and expansion are disabled"; + case "outside": + return "Symbolic link outside the workspace; opening is disabled"; + case "broken": + return "Broken symbolic link; opening is disabled"; + case "other": + return "Symbolic link to an unsupported filesystem entry; opening is disabled"; + default: + return "Symbolic link with an unknown target"; + } +} + +function createReviewFileTree(): ReviewFileTree { + let state: ReviewFileTree; + const model = new FileTree({ + paths: [], + initialExpansion: "closed", + flattenEmptyDirectories: true, + stickyFolders: true, + renderRowDecoration: ({ item }) => { + const entry = state.entryByPath.get(item.path); + const title = entry === undefined ? null : symlinkDescription(entry); + return title === null ? null : { text: "↗", title }; + }, + }); + state = { + model, + entries: [], + entryByPath: new Map(), + directoryPaths: new Set(), + }; + return state; +} + +export function getReviewFileTree(sessionId: string): ReviewFileTree { + const existing = reviewTrees.get(sessionId); + if (existing !== undefined) return existing; + const created = createReviewFileTree(); + reviewTrees.set(sessionId, created); + return created; +} + +export function unionDeletedReviewEntries( + entries: ReadonlyArray, + files: ReadonlyArray, +): ReadonlyArray { + const existing = new Set(entries.map((entry) => entry.path)); + const extra: WorkspaceTreeEntry[] = []; + for (const file of files) { + if (file.status !== "deleted") continue; + const parts = file.path.split("/").filter((part) => part !== ""); + let prefix = ""; + for (const [index, part] of parts.entries()) { + prefix = prefix === "" ? part : `${prefix}/${part}`; + if (existing.has(prefix)) continue; + existing.add(prefix); + extra.push( + index === parts.length - 1 + ? { path: prefix, type: "file" } + : { path: prefix, type: "directory" }, + ); + } + } + return extra.length === 0 ? entries : [...entries, ...extra]; +} + +export function syncReviewFileTree( + state: ReviewFileTree, + entries: ReadonlyArray, + gitStatus: ReadonlyArray<{ path: string; status: "added" | "deleted" | "modified" | "renamed" }>, +): void { + if (state.entries !== entries) { + const expandedPaths: string[] = []; + for (const directoryPath of state.directoryPaths) { + const item = state.model.getItem(directoryPath); + if (item !== null && item.isDirectory() && "isExpanded" in item && item.isExpanded()) { + expandedPaths.push(directoryPath); + } + } + + const paths: string[] = []; + const nextDirectoryPaths = new Set(); + const entryByPath = new Map(); + for (const entry of entries) { + const pierrePath = toPierrePath(entry); + paths.push(pierrePath); + if (entry.type === "directory") nextDirectoryPaths.add(pierrePath); + entryByPath.set(entry.path, entry); + } + const preparedInput = prepareFileTreeInput(paths); + state.entries = entries; + state.entryByPath = entryByPath; + state.directoryPaths = nextDirectoryPaths; + state.model.resetPaths({ + preparedInput, + initialExpandedPaths: expandedPaths.filter((path) => nextDirectoryPaths.has(path)), + }); + } + state.model.setGitStatus(gitStatus); +} + +export function isOpenableTreeEntry(entry: WorkspaceTreeEntry | undefined): boolean { + return entry?.type === "file" || (entry?.type === "symlink" && entry.symlinkTarget === "file"); +} + +if (typeof window !== "undefined") { + window.addEventListener( + "beforeunload", + () => { + for (const state of reviewTrees.values()) state.model.cleanUp(); + reviewTrees.clear(); + }, + { once: true }, + ); +} diff --git a/apps/app/src/features/review/review-workspace-layout.tsx b/apps/app/src/features/review/review-workspace-layout.tsx index 8cb5ef3ed..b310fe9a0 100644 --- a/apps/app/src/features/review/review-workspace-layout.tsx +++ b/apps/app/src/features/review/review-workspace-layout.tsx @@ -1,17 +1,19 @@ import { Button } from "@vibest/ui/components/button"; import { Sheet, SheetHeader, SheetPopup, SheetTitle } from "@vibest/ui/components/sheet"; import { useIsMobile } from "@vibest/ui/hooks/use-media-query"; -import { ListTreeIcon } from "lucide-react"; +import { FilesIcon } from "lucide-react"; import { type ReactNode, useId, useLayoutEffect, useRef, useState } from "react"; import { Group, Panel, Separator } from "react-resizable-panels"; const MIN_SPLIT_WIDTH = 24 * 16 + 6; export function ReviewWorkspaceLayout({ + toolbar, preview, files, filesLabel, }: { + toolbar: ReactNode; preview: ReactNode; files: ReactNode; filesLabel: string; @@ -42,24 +44,28 @@ export function ReviewWorkspaceLayout({ return (
    - {useDrawer ? ( - <> - {preview} +
    + {toolbar} + {useDrawer ? ( + ) : null} +
    + {useDrawer ? ( + <> + {preview} - Changed files + Project files
    {files}
     @@ -79,7 +85,7 @@ export function ReviewWorkspaceLayout({ {preview} ; +export type GitBranchQuery = ReturnType; diff --git a/apps/app/src/features/review/use-git-diffs.ts b/apps/app/src/features/review/use-git-diffs.ts new file mode 100644 index 000000000..72cc552b0 --- /dev/null +++ b/apps/app/src/features/review/use-git-diffs.ts @@ -0,0 +1,31 @@ +import { skipToken, useQueries } from "@tanstack/react-query"; +import { useRouteContext } from "@tanstack/react-router"; +import type { GitReviewFile, GitReviewMode } from "@vibest/contract/git"; + +export function useGitDiffs( + cwd: string | undefined, + files: ReadonlyArray, + mode: GitReviewMode, + other: string | undefined, +) { + const { orpcQueryUtils } = useRouteContext({ from: "__root__" }); + return useQueries({ + queries: files.map((file) => ({ + ...orpcQueryUtils.git.diff.queryOptions({ + input: + cwd === undefined + ? skipToken + : { + cwd, + path: file.path, + mode, + ...(mode === "branch" && other !== undefined ? { other } : {}), + }, + }), + refetchOnWindowFocus: "always" as const, + staleTime: Infinity, + })), + }); +} + +export type GitDiffsQuery = ReturnType; diff --git a/apps/app/src/features/review/use-git-review.ts b/apps/app/src/features/review/use-git-review.ts index 638ea8188..cceaed0b4 100644 --- a/apps/app/src/features/review/use-git-review.ts +++ b/apps/app/src/features/review/use-git-review.ts @@ -1,12 +1,25 @@ import { skipToken, useQuery } from "@tanstack/react-query"; import { useRouteContext } from "@tanstack/react-router"; +import type { GitReviewMode } from "@vibest/contract/git"; -export function useGitReview(cwd: string | undefined) { +export function useGitReview( + cwd: string | undefined, + mode: GitReviewMode, + other: string | undefined, +) { const { orpcQueryUtils } = useRouteContext({ from: "__root__" }); return useQuery({ ...orpcQueryUtils.git.review.queryOptions({ - input: cwd === undefined ? skipToken : { cwd }, + input: + cwd === undefined + ? skipToken + : { + cwd, + mode, + ...(mode === "branch" && other !== undefined ? { other } : {}), + }, }), + enabled: cwd !== undefined && (mode !== "branch" || other !== undefined), refetchOnWindowFocus: "always", staleTime: Infinity, }); diff --git a/apps/app/src/features/review/use-workspace-tree.ts b/apps/app/src/features/review/use-workspace-tree.ts new file mode 100644 index 000000000..ab19b347a --- /dev/null +++ b/apps/app/src/features/review/use-workspace-tree.ts @@ -0,0 +1,15 @@ +import { skipToken, useQuery } from "@tanstack/react-query"; +import { useRouteContext } from "@tanstack/react-router"; + +export function useWorkspaceTree(cwd: string | undefined) { + const { orpcQueryUtils } = useRouteContext({ from: "__root__" }); + return useQuery({ + ...orpcQueryUtils.fs.readTree.queryOptions({ + input: cwd === undefined ? skipToken : { cwd }, + }), + refetchOnWindowFocus: "always", + staleTime: Infinity, + }); +} + +export type WorkspaceTreeQuery = ReturnType; diff --git a/packages/contract/src/git.ts b/packages/contract/src/git.ts index 7ae3a3c3c..396e39f99 100644 --- a/packages/contract/src/git.ts +++ b/packages/contract/src/git.ts @@ -4,14 +4,11 @@ import { Schema } from "effect"; import { toStandardSchema } from "./domain"; const CwdInput = Schema.Struct({ cwd: Schema.String }); -const CwdPathInput = Schema.Struct({ - cwd: Schema.String, - path: Schema.String, -}); const pathData = toStandardSchema(Schema.Struct({ path: Schema.String })); const pathEscapeData = toStandardSchema(Schema.Struct({ cwd: Schema.String, path: Schema.String })); const cwdData = toStandardSchema(Schema.Struct({ cwd: Schema.String })); +const refData = toStandardSchema(Schema.Struct({ ref: Schema.String })); export const GitStatusFileSchema = Schema.Struct({ path: Schema.String, @@ -29,11 +26,31 @@ export type GitStatus = typeof GitStatusSchema.Type; export const GitBranchSchema = Schema.Struct({ current: Schema.Union([Schema.String, Schema.Null]), + /** Preferred compare target: `origin/main` when `origin/HEAD` exists, else local `main`. */ defaultBranch: Schema.Union([Schema.String, Schema.Null]), + /** Local heads and remote-tracking refs (`main`, `origin/main`). */ branches: Schema.Array(Schema.String), }); export type GitBranch = typeof GitBranchSchema.Type; +export const GitReviewModeSchema = Schema.Literals(["uncommitted", "committed", "branch"]); +export type GitReviewMode = typeof GitReviewModeSchema.Type; + +export const GitReviewQuerySchema = Schema.Struct({ + cwd: Schema.String, + mode: Schema.optionalKey(GitReviewModeSchema), + other: Schema.optionalKey(Schema.String), +}); +export type GitReviewQuery = typeof GitReviewQuerySchema.Type; + +export const GitDiffQuerySchema = Schema.Struct({ + cwd: Schema.String, + path: Schema.String, + mode: Schema.optionalKey(GitReviewModeSchema), + other: Schema.optionalKey(Schema.String), +}); +export type GitDiffQuery = typeof GitDiffQuerySchema.Type; + export const GitReviewFileStatusSchema = Schema.Literals([ "modified", "added", @@ -51,11 +68,13 @@ export const GitReviewFileSchema = Schema.Struct({ export type GitReviewFile = typeof GitReviewFileSchema.Type; /** - * A local review against the integration branch: three-dot - * `merge-base(default, HEAD)` plus the working tree. On the default branch - * (or with no default) the base is `HEAD`, so the set is uncommitted only. + * A change set for the Review panel. `mode` chooses uncommitted vs HEAD, + * committed three-dot vs the default branch, or three-dot vs `other` + * (a local or remote-tracking ref). */ export const GitReviewSchema = Schema.Struct({ + mode: GitReviewModeSchema, + other: Schema.Union([Schema.String, Schema.Null]), branch: Schema.Union([Schema.String, Schema.Null]), base: Schema.String, baseBranch: Schema.Union([Schema.String, Schema.Null]), @@ -80,8 +99,13 @@ const cwdErrors = { GIT_FAILED: { data: cwdData }, }; -const diffErrors = { +const reviewErrors = { ...cwdErrors, + REF_NOT_FOUND: { data: refData }, +}; + +const diffErrors = { + ...reviewErrors, NOT_FOUND: { data: pathData }, BINARY_FILE: { data: pathData }, FILE_TOO_LARGE: { @@ -93,8 +117,7 @@ const diffErrors = { /** * Read-only git. Callers pass `cwd`; the server confines paths to that - * workspace and never writes. `review` / `diff` are the Code Review Panel - * surface — a change set vs the default branch, not staged/unstaged buckets. + * workspace and never writes. `review` / `diff` take an explicit compare mode. */ export const gitContract = { status: oc @@ -106,11 +129,11 @@ export const gitContract = { .errors(cwdErrors) .output(toStandardSchema(GitBranchSchema)), review: oc - .input(toStandardSchema(CwdInput)) - .errors(cwdErrors) + .input(toStandardSchema(GitReviewQuerySchema)) + .errors(reviewErrors) .output(toStandardSchema(GitReviewSchema)), diff: oc - .input(toStandardSchema(CwdPathInput)) + .input(toStandardSchema(GitDiffQuerySchema)) .errors(diffErrors) .output(toStandardSchema(GitFileDiffSchema)), }; diff --git a/packages/server/src/errors.ts b/packages/server/src/errors.ts index ada63e399..8237d78c3 100644 --- a/packages/server/src/errors.ts +++ b/packages/server/src/errors.ts @@ -26,6 +26,11 @@ export class GitNotRepository extends Data.TaggedError("GitNotRepository")<{ readonly cwd: string; }> {} +/** `other` is not a local head or remote-tracking ref in this repository. */ +export class GitRefNotFound extends Data.TaggedError("GitRefNotFound")<{ + readonly ref: string; +}> {} + export class SessionNotFound extends Data.TaggedError("SessionNotFound")<{ readonly projectId: string; readonly sessionId: string; diff --git a/packages/server/src/git/service.ts b/packages/server/src/git/service.ts index 9054db9ef..e149f8c87 100644 --- a/packages/server/src/git/service.ts +++ b/packages/server/src/git/service.ts @@ -2,9 +2,12 @@ import path from "node:path"; import type { GitBranch, + GitDiffQuery, GitFileDiff, GitReview, GitReviewFile, + GitReviewMode, + GitReviewQuery, GitStatus, GitStatusFile, } from "@vibest/contract/git"; @@ -14,6 +17,7 @@ import { simpleGit } from "simple-git"; import { GitError, GitNotRepository, + GitRefNotFound, WorkspaceBinaryFile, WorkspaceFileNotFound, WorkspaceFileTooLarge, @@ -75,6 +79,17 @@ const decodeText = ( } }; +/** Reject anything that is not a listed ref name — no `../`, flags, or rev magic. */ +const isUnsafeRef = (ref: string): boolean => + ref === "" || + ref.startsWith("-") || + ref.includes("..") || + ref.includes("\\") || + ref.includes("\0") || + ref.includes(":") || + ref.includes("@{") || + /\s/.test(ref); + type GitFailure = | WorkspacePathEscape | WorkspaceNotDirectory @@ -82,13 +97,24 @@ type GitFailure = | GitNotRepository | GitError; +type GitReviewFailure = GitFailure | GitRefNotFound; + type GitDiffFailure = - | GitFailure + | GitReviewFailure | WorkspaceFileNotFound | WorkspaceNotFile | WorkspaceBinaryFile | WorkspaceFileTooLarge; +type ComparePlan = { + readonly mode: GitReviewMode; + readonly other: string | null; + readonly base: string; + readonly baseBranch: string | null; + readonly head: string | null; + readonly includeUntracked: boolean; +}; + /** * Read-only `git` module. Workspace confinement matches `FileSystemService`: * `cwd` must be an absolute directory, and every path git reports is rewritten @@ -99,8 +125,8 @@ export class GitService extends Context.Service< { readonly status: (cwd: string) => Effect.Effect; readonly branch: (cwd: string) => Effect.Effect; - readonly review: (cwd: string) => Effect.Effect; - readonly diff: (cwd: string, path: string) => Effect.Effect; + readonly review: (query: GitReviewQuery) => Effect.Effect; + readonly diff: (query: GitDiffQuery) => Effect.Effect; } >()("GitService") {} @@ -165,7 +191,25 @@ export const GitServiceLayer: Layer.Layer< return { path: nextPath, status: file.status, oldPath }; }; - const resolveDefaultRef = (cwd: string) => + const parseRefNames = (output: string): string[] => { + const names: string[] = []; + for (const line of output.split("\n")) { + const ref = line.trim(); + if (ref.startsWith("refs/heads/")) { + names.push(ref.slice("refs/heads/".length)); + } else if (ref.startsWith("refs/remotes/")) { + names.push(ref.slice("refs/remotes/".length)); + } + } + return names; + }; + + const listRefs = (cwd: string) => + raw(cwd, ["for-each-ref", "--format=%(refname)", "refs/heads", "refs/remotes"]).pipe( + Effect.map(parseRefNames), + ); + + const resolvePreferredCompareRef = (cwd: string) => Effect.gen(function* () { const remoteHead = yield* raw(cwd, [ "symbolic-ref", @@ -178,39 +222,73 @@ export const GitServiceLayer: Layer.Layer< if (remoteHead.startsWith("refs/remotes/")) { return remoteHead.slice("refs/remotes/".length); } - const local = yield* raw(cwd, ["for-each-ref", "--format=%(refname:short)", "refs/heads"]); - const names = new Set( - local - .split("\n") - .map((name) => name.trim()) - .filter(Boolean), - ); + const local = yield* raw(cwd, ["for-each-ref", "--format=%(refname)", "refs/heads"]); + const names = new Set(parseRefNames(local)); for (const name of DEFAULT_BRANCH_NAMES) { if (names.has(name)) return name; } return null; }); - const shortBranchName = (ref: string): string => ref.replace(/^origin\//, ""); - - const isOnDefault = (current: string | null, defaultRef: string): boolean => { - if (current === null || current === "HEAD") return false; - return current === defaultRef || current === shortBranchName(defaultRef); - }; + const mergeBase = (cwd: string, other: string) => + raw(cwd, ["merge-base", "HEAD", other]).pipe( + Effect.map((value) => value.trim()), + Effect.flatMap((sha) => + sha === "" + ? Effect.fail(new GitError({ cwd, cause: `empty merge-base with ${other}` })) + : Effect.succeed(sha), + ), + ); - const resolveReviewBase = (cwd: string, current: string | null) => + const resolveCompare = ( + cwd: string, + query: { readonly mode?: GitReviewMode; readonly other?: string }, + ): Effect.Effect => Effect.gen(function* () { - const defaultRef = yield* resolveDefaultRef(cwd); - if (defaultRef === null || isOnDefault(current, defaultRef)) { - return { base: "HEAD", baseBranch: null as string | null }; + const mode = query.mode ?? "uncommitted"; + if (mode === "uncommitted") { + return { + mode, + other: null, + base: "HEAD", + baseBranch: null, + head: null, + includeUntracked: true, + }; } - const mergeBase = yield* raw(cwd, ["merge-base", "HEAD", defaultRef]).pipe( - Effect.map((value) => value.trim()), - Effect.catch(() => Effect.succeed("")), - ); + + if (mode === "committed") { + const defaultRef = yield* resolvePreferredCompareRef(cwd); + if (defaultRef === null) { + return yield* new GitError({ cwd, cause: "no default branch to compare" }); + } + const base = yield* mergeBase(cwd, defaultRef); + return { + mode, + other: null, + base, + baseBranch: defaultRef, + head: "HEAD", + includeUntracked: false, + }; + } + + const other = query.other; + if (other === undefined || isUnsafeRef(other)) { + return yield* new GitRefNotFound({ ref: other ?? "" }); + } + const refs = yield* listRefs(cwd); + if (!refs.includes(other)) { + return yield* new GitRefNotFound({ ref: other }); + } + const base = yield* mergeBase(cwd, other); return { - base: mergeBase === "" ? defaultRef : mergeBase, - baseBranch: shortBranchName(defaultRef), + mode, + other, + base, + baseBranch: other, + head: null, + includeUntracked: true, }; }); @@ -222,20 +300,31 @@ export const GitServiceLayer: Layer.Layer< }), ); - const reviewFiles = (cwd: string, repoRoot: string, base: string) => + const reviewFiles = (cwd: string, repoRoot: string, plan: ComparePlan) => Effect.gen(function* () { - const nameStatus = yield* raw(cwd, ["diff", "--name-status", "-z", "--find-renames", base]); + const diffArgs = + plan.head === null + ? ["diff", "--name-status", "-z", "--find-renames", plan.base] + : ["diff", "--name-status", "-z", "--find-renames", plan.base, plan.head]; + const nameStatus = yield* raw(cwd, diffArgs); const tracked = parseNameStatus(nameStatus) .map((file) => relocate(cwd, repoRoot, file)) .filter((file): file is GitReviewFile => file !== null); - const untrackedRaw = yield* raw(cwd, ["ls-files", "-z", "--others", "--exclude-standard"]); - const seen = new Set(tracked.map((file) => file.path)); const files = [...tracked]; - for (const gitPath of parseNulPaths(untrackedRaw)) { - const nextPath = toWorkspacePath(cwd, repoRoot, gitPath); - if (nextPath === null || seen.has(nextPath)) continue; - seen.add(nextPath); - files.push({ path: nextPath, status: "added" }); + const seen = new Set(tracked.map((file) => file.path)); + if (plan.includeUntracked) { + const untrackedRaw = yield* raw(cwd, [ + "ls-files", + "-z", + "--others", + "--exclude-standard", + ]); + for (const gitPath of parseNulPaths(untrackedRaw)) { + const nextPath = toWorkspacePath(cwd, repoRoot, gitPath); + if (nextPath === null || seen.has(nextPath)) continue; + seen.add(nextPath); + files.push({ path: nextPath, status: "added" }); + } } files.sort((left, right) => left.path.localeCompare(right.path, undefined, { numeric: true, sensitivity: "base" }), @@ -246,9 +335,12 @@ export const GitServiceLayer: Layer.Layer< const readWorktreeText = (cwd: string, relativePath: string) => workspace.readFileString(cwd, relativePath); - const readBlobText = (cwd: string, base: string, blobPath: string) => + const toBlobPath = (cwd: string, repoRoot: string, relativePath: string): string => + toPosixPath(path.relative(repoRoot, path.resolve(cwd, relativePath))); + + const readBlobText = (cwd: string, treeish: string, blobPath: string) => Effect.gen(function* () { - const sizeRaw = yield* raw(cwd, ["cat-file", "-s", `${base}:${blobPath}`]).pipe( + const sizeRaw = yield* raw(cwd, ["cat-file", "-s", `${treeish}:${blobPath}`]).pipe( Effect.catch(() => Effect.succeed("")), ); if (sizeRaw.trim() === "") return null; @@ -260,7 +352,7 @@ export const GitServiceLayer: Layer.Layer< limit: MAX_FILE_BYTES, }); } - const text = yield* raw(cwd, ["cat-file", "-p", `${base}:${blobPath}`]); + const text = yield* raw(cwd, ["cat-file", "-p", `${treeish}:${blobPath}`]); const bytes = new TextEncoder().encode(text); if (bytes.byteLength > MAX_FILE_BYTES) { return yield* new WorkspaceFileTooLarge({ @@ -307,55 +399,51 @@ export const GitServiceLayer: Layer.Layer< Effect.gen(function* () { const realRoot = yield* resolveRoot(cwd); const current = yield* currentBranch(realRoot); - const defaultRef = yield* resolveDefaultRef(realRoot); - const listed = yield* raw(realRoot, [ - "for-each-ref", - "--format=%(refname:short)", - "refs/heads", - ]); - const branches = listed - .split("\n") - .map((name) => name.trim()) - .filter(Boolean); - return { - current, - defaultBranch: defaultRef === null ? null : shortBranchName(defaultRef), - branches, - }; + const defaultBranch = yield* resolvePreferredCompareRef(realRoot); + const branches = yield* listRefs(realRoot); + return { current, defaultBranch, branches }; }), - review: (cwd) => + review: (query) => Effect.gen(function* () { - const realRoot = yield* resolveRoot(cwd); + const realRoot = yield* resolveRoot(query.cwd); const repoRoot = yield* resolveRepoRoot(realRoot); const branch = yield* currentBranch(realRoot); - const { base, baseBranch } = yield* resolveReviewBase(realRoot, branch); - const files = yield* reviewFiles(realRoot, repoRoot, base); - return { branch, base, baseBranch, files }; + const plan = yield* resolveCompare(realRoot, query); + const files = yield* reviewFiles(realRoot, repoRoot, plan); + return { + mode: plan.mode, + other: plan.other, + branch, + base: plan.base, + baseBranch: plan.baseBranch, + files, + }; }), - diff: (cwd, relativePath) => + diff: (query) => Effect.gen(function* () { - const realRoot = yield* resolveRoot(cwd); - if (path.isAbsolute(relativePath) || relativePath.split(/[\\/]/).includes("..")) { - return yield* new WorkspacePathEscape({ cwd: realRoot, path: relativePath }); + const realRoot = yield* resolveRoot(query.cwd); + if (path.isAbsolute(query.path) || query.path.split(/[\\/]/).includes("..")) { + return yield* new WorkspacePathEscape({ cwd: realRoot, path: query.path }); } const repoRoot = yield* resolveRepoRoot(realRoot); - const branch = yield* currentBranch(realRoot); - const { base } = yield* resolveReviewBase(realRoot, branch); - const files = yield* reviewFiles(realRoot, repoRoot, base); - const file = files.find((entry) => entry.path === relativePath); + const plan = yield* resolveCompare(realRoot, query); + const files = yield* reviewFiles(realRoot, repoRoot, plan); + const file = files.find((entry) => entry.path === query.path); if (file === undefined) { - return yield* new WorkspaceFileNotFound({ path: relativePath }); + return yield* new WorkspaceFileNotFound({ path: query.path }); } - const workspaceBlob = file.oldPath ?? file.path; - const blobPath = toPosixPath( - path.relative(repoRoot, path.resolve(realRoot, workspaceBlob)), - ); + const oldBlobPath = toBlobPath(realRoot, repoRoot, file.oldPath ?? file.path); + const newBlobPath = toBlobPath(realRoot, repoRoot, file.path); const oldContents = - file.status === "added" ? null : yield* readBlobText(realRoot, base, blobPath); + file.status === "added" ? null : yield* readBlobText(realRoot, plan.base, oldBlobPath); const newContents = - file.status === "deleted" ? null : yield* readWorktreeText(realRoot, file.path); + file.status === "deleted" + ? null + : plan.head === null + ? yield* readWorktreeText(realRoot, file.path) + : yield* readBlobText(realRoot, plan.head, newBlobPath); return { path: file.path, status: file.status, diff --git a/packages/server/src/rpc/git.ts b/packages/server/src/rpc/git.ts index cdc706073..b236be0f9 100644 --- a/packages/server/src/rpc/git.ts +++ b/packages/server/src/rpc/git.ts @@ -41,7 +41,7 @@ export const gitRouter = orpc.router({ }), review: orpc.review.effect(function* ({ input, errors }) { const git = yield* GitService; - return yield* git.review(input.cwd).pipe( + return yield* git.review(input).pipe( Effect.catchTags({ WorkspacePathEscape: (error) => Effect.fail(errors.PATH_ESCAPE({ data: { cwd: error.cwd, path: error.path } })), @@ -51,12 +51,13 @@ export const gitRouter = orpc.router({ GitNotRepository: (error) => Effect.fail(errors.NOT_REPOSITORY({ data: { cwd: error.cwd } })), GitError: (error) => Effect.fail(errors.GIT_FAILED({ data: { cwd: error.cwd } })), + GitRefNotFound: (error) => Effect.fail(errors.REF_NOT_FOUND({ data: { ref: error.ref } })), }), ); }), diff: orpc.diff.effect(function* ({ input, errors }) { const git = yield* GitService; - return yield* git.diff(input.cwd, input.path).pipe( + return yield* git.diff(input).pipe( Effect.catchTags({ WorkspacePathEscape: (error) => Effect.fail(errors.PATH_ESCAPE({ data: { cwd: error.cwd, path: error.path } })), @@ -66,6 +67,7 @@ export const gitRouter = orpc.router({ GitNotRepository: (error) => Effect.fail(errors.NOT_REPOSITORY({ data: { cwd: error.cwd } })), GitError: (error) => Effect.fail(errors.GIT_FAILED({ data: { cwd: error.cwd } })), + GitRefNotFound: (error) => Effect.fail(errors.REF_NOT_FOUND({ data: { ref: error.ref } })), WorkspaceFileNotFound: (error) => Effect.fail(errors.NOT_FOUND({ data: { path: error.path } })), WorkspaceNotFile: (error) => Effect.fail(errors.NOT_FOUND({ data: { path: error.path } })), diff --git a/packages/server/test/git.test.ts b/packages/server/test/git.test.ts index 48539c557..ac2ef9853 100644 --- a/packages/server/test/git.test.ts +++ b/packages/server/test/git.test.ts @@ -28,6 +28,14 @@ layer(NodePlatformLayer)("GitService", (it) => { return dir; }); + const addRemoteMain = (dir: string) => + Effect.promise(async () => { + const git = simpleGit(dir); + const sha = (await git.revparse(["main"])).trim(); + await git.raw(["update-ref", "refs/remotes/origin/main", sha]); + await git.raw(["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/main"]); + }); + it.effect("reports working-tree status with untracked files", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -41,7 +49,7 @@ layer(NodePlatformLayer)("GitService", (it) => { }).pipe(Effect.provide(GitLayer)), ); - it.effect("lists branches and the default branch", () => + it.effect("lists branches and the local default branch", () => Effect.gen(function* () { const dir = yield* repo; const git = yield* GitService; @@ -52,7 +60,21 @@ layer(NodePlatformLayer)("GitService", (it) => { }).pipe(Effect.provide(GitLayer)), ); - it.effect("reviews uncommitted work on the default branch against HEAD", () => + it.effect("lists local and remote-tracking refs without fetching", () => + Effect.gen(function* () { + const dir = yield* repo; + yield* addRemoteMain(dir); + const git = yield* GitService; + const branch = yield* git.branch(dir); + assert.equal(branch.current, "main"); + assert.equal(branch.defaultBranch, "origin/main"); + assert.ok(branch.branches.includes("main")); + assert.ok(branch.branches.includes("origin/main")); + assert.ok(branch.branches.includes("origin/HEAD")); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("defaults review to uncommitted work against HEAD", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const dir = yield* repo; @@ -60,7 +82,9 @@ layer(NodePlatformLayer)("GitService", (it) => { yield* fs.writeFileString(path.join(dir, "added.txt"), "new\n"); const git = yield* GitService; - const review = yield* git.review(dir); + const review = yield* git.review({ cwd: dir }); + assert.equal(review.mode, "uncommitted"); + assert.equal(review.other, null); assert.equal(review.branch, "main"); assert.equal(review.base, "HEAD"); assert.equal(review.baseBranch, null); @@ -69,19 +93,19 @@ layer(NodePlatformLayer)("GitService", (it) => { "added.txt", ]); - const modified = yield* git.diff(dir, "a.txt"); + const modified = yield* git.diff({ cwd: dir, path: "a.txt" }); assert.equal(modified.status, "modified"); assert.equal(modified.oldContents, "hi\n"); assert.equal(modified.newContents, "hello\n"); - const added = yield* git.diff(dir, "added.txt"); + const added = yield* git.diff({ cwd: dir, path: "added.txt" }); assert.equal(added.status, "added"); assert.equal(added.oldContents, null); assert.equal(added.newContents, "new\n"); }).pipe(Effect.provide(GitLayer)), ); - it.effect("reviews a feature branch against merge-base with main", () => + it.effect("committed mode diffs HEAD against merge-base and ignores the worktree", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const dir = yield* repo; @@ -95,23 +119,25 @@ layer(NodePlatformLayer)("GitService", (it) => { await git.add("feature.txt"); await git.commit("feature work"); }); + yield* fs.writeFileString(path.join(dir, "wip.txt"), "uncommitted\n"); const git = yield* GitService; - const review = yield* git.review(dir); + const review = yield* git.review({ cwd: dir, mode: "committed" }); + assert.equal(review.mode, "committed"); assert.equal(review.branch, "feature"); assert.equal(review.baseBranch, "main"); assert.notEqual(review.base, "HEAD"); - assert.ok( - review.files.some((file) => file.path === "feature.txt" && file.status === "added"), - ); + assert.deepEqual(Array.from(review.files.map((file) => file.path)).toSorted(), [ + "feature.txt", + ]); - const diff = yield* git.diff(dir, "feature.txt"); + const diff = yield* git.diff({ cwd: dir, mode: "committed", path: "feature.txt" }); assert.equal(diff.oldContents, null); assert.equal(diff.newContents, "branch\n"); }).pipe(Effect.provide(GitLayer)), ); - it.effect("ignores later main commits when reviewing a feature branch", () => + it.effect("branch mode includes uncommitted files against a local or remote ref", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const dir = yield* repo; @@ -132,18 +158,55 @@ layer(NodePlatformLayer)("GitService", (it) => { const git = simpleGit(dir); await git.add(["a.txt", "extra.txt"]); await git.commit("main moved forward"); + const sha = (await git.revparse(["main"])).trim(); + await git.raw(["update-ref", "refs/remotes/origin/main", sha]); await git.checkout("feature"); }); yield* fs.writeFileString(path.join(dir, "wip.txt"), "uncommitted\n"); const git = yield* GitService; - const review = yield* git.review(dir); - assert.equal(review.branch, "feature"); - assert.equal(review.baseBranch, "main"); - assert.deepEqual(Array.from(review.files.map((file) => file.path)).toSorted(), [ + const uncommitted = yield* git.review({ cwd: dir }); + assert.equal(uncommitted.mode, "uncommitted"); + assert.deepEqual(Array.from(uncommitted.files.map((file) => file.path)).toSorted(), [ + "wip.txt", + ]); + + const vsMain = yield* git.review({ cwd: dir, mode: "branch", other: "main" }); + assert.equal(vsMain.mode, "branch"); + assert.equal(vsMain.other, "main"); + assert.equal(vsMain.baseBranch, "main"); + assert.deepEqual(Array.from(vsMain.files.map((file) => file.path)).toSorted(), [ "feature.txt", "wip.txt", ]); + + const vsOrigin = yield* git.review({ cwd: dir, mode: "branch", other: "origin/main" }); + assert.equal(vsOrigin.other, "origin/main"); + assert.deepEqual(Array.from(vsOrigin.files.map((file) => file.path)).toSorted(), [ + "feature.txt", + "wip.txt", + ]); + }).pipe(Effect.provide(GitLayer)), + ); + + it.effect("rejects an unknown or missing compare ref", () => + Effect.gen(function* () { + const dir = yield* repo; + const git = yield* GitService; + + const missingOther = yield* git.review({ cwd: dir, mode: "branch" }).pipe(Effect.flip); + assert.equal(missingOther._tag, "GitRefNotFound"); + + const unknown = yield* git + .review({ cwd: dir, mode: "branch", other: "no-such-branch" }) + .pipe(Effect.flip); + assert.equal(unknown._tag, "GitRefNotFound"); + if (unknown._tag === "GitRefNotFound") assert.equal(unknown.ref, "no-such-branch"); + + const unsafe = yield* git + .review({ cwd: dir, mode: "branch", other: "../main" }) + .pipe(Effect.flip); + assert.equal(unsafe._tag, "GitRefNotFound"); }).pipe(Effect.provide(GitLayer)), ); @@ -154,7 +217,7 @@ layer(NodePlatformLayer)("GitService", (it) => { yield* fs.remove(path.join(dir, "a.txt")); const git = yield* GitService; - const diff = yield* git.diff(dir, "a.txt"); + const diff = yield* git.diff({ cwd: dir, path: "a.txt" }); assert.equal(diff.status, "deleted"); assert.equal(diff.oldContents, "hi\n"); assert.equal(diff.newContents, null); @@ -170,7 +233,7 @@ layer(NodePlatformLayer)("GitService", (it) => { const relative = yield* git.status("relative/workspace").pipe(Effect.flip); assert.equal(relative._tag, "WorkspacePathEscape"); - const missing = yield* git.review(dir).pipe(Effect.flip); + const missing = yield* git.review({ cwd: dir }).pipe(Effect.flip); assert.equal(missing._tag, "GitNotRepository"); }).pipe(Effect.provide(GitLayer)), ); @@ -179,7 +242,7 @@ layer(NodePlatformLayer)("GitService", (it) => { Effect.gen(function* () { const dir = yield* repo; const git = yield* GitService; - const missing = yield* git.diff(dir, "nope.ts").pipe(Effect.flip); + const missing = yield* git.diff({ cwd: dir, path: "nope.ts" }).pipe(Effect.flip); assert.equal(missing._tag, "WorkspaceFileNotFound"); }).pipe(Effect.provide(GitLayer)), ); diff --git a/packages/server/test/rpc-git.test.ts b/packages/server/test/rpc-git.test.ts index 12f01637a..a33ca5c24 100644 --- a/packages/server/test/rpc-git.test.ts +++ b/packages/server/test/rpc-git.test.ts @@ -27,6 +27,8 @@ describe("git router", () => { const harness = await makeRpcTestHarness(home); try { const review = await harness.client.git.review({ cwd }); + expect(review.mode).toBe("uncommitted"); + expect(review.other).toBeNull(); expect(review.branch).toBe("main"); expect(review.base).toBe("HEAD"); expect(review.baseBranch).toBeNull(); @@ -40,6 +42,22 @@ describe("git router", () => { } }); + it("maps a missing compare ref to REF_NOT_FOUND", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-home-")); + const cwd = await makeRepo(); + const harness = await makeRpcTestHarness(home); + try { + await expect( + harness.client.git.review({ cwd, mode: "branch", other: "nope" }), + ).rejects.toMatchObject({ + code: "REF_NOT_FOUND", + data: { ref: "nope" }, + }); + } finally { + await harness.dispose(); + } + }); + it("maps a non-repository and a relative cwd to typed errors", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-home-")); const dir = fs.mkdtempSync(path.join(os.tmpdir(), "vibest-not-git-")); From c91d4f14333b1ccca882a873b6fb58f94609867e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 14:21:51 +0000 Subject: [PATCH 3/8] fix: group vs-branch remotes from git refs, not slashes Local branches like feature/oauth were landing in the Remote group because the toolbar split on `/`. git.branch now returns remotes separately so the select can keep origin/main remote without mislabeling slashed local names. --- .../review/review-file-status.test.ts | 11 +++++--- .../src/features/review/review-file-status.ts | 8 ++++-- .../src/features/review/review-toolbar.tsx | 2 +- packages/contract/src/git.ts | 2 ++ packages/server/src/git/service.ts | 25 ++++++++++++++++--- packages/server/test/git.test.ts | 4 +++ 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/apps/app/src/features/review/review-file-status.test.ts b/apps/app/src/features/review/review-file-status.test.ts index e644bcb67..c72800b54 100644 --- a/apps/app/src/features/review/review-file-status.test.ts +++ b/apps/app/src/features/review/review-file-status.test.ts @@ -62,9 +62,14 @@ describe("isReviewMode", () => { }); describe("splitCompareRefs", () => { - it("keeps local and remote-tracking names in separate groups", () => { - expect(splitCompareRefs(["main", "feature", "origin/main", "origin/HEAD"])).toEqual({ - local: ["main", "feature"], + it("keeps slashed local names out of the remote group", () => { + expect( + splitCompareRefs( + ["main", "feature/oauth", "origin/main", "origin/HEAD"], + ["origin/main", "origin/HEAD"], + ), + ).toEqual({ + local: ["main", "feature/oauth"], remote: ["origin/main", "origin/HEAD"], }); }); diff --git a/apps/app/src/features/review/review-file-status.ts b/apps/app/src/features/review/review-file-status.ts index 8d91eb2b5..4687b32d4 100644 --- a/apps/app/src/features/review/review-file-status.ts +++ b/apps/app/src/features/review/review-file-status.ts @@ -71,14 +71,18 @@ export function emptyReviewMessage(review: { } } -export function splitCompareRefs(branches: ReadonlyArray): { +export function splitCompareRefs( + branches: ReadonlyArray, + remotes: ReadonlyArray, +): { local: string[]; remote: string[]; } { + const remoteSet = new Set(remotes); const local: string[] = []; const remote: string[] = []; for (const name of branches) { - if (name.includes("/")) remote.push(name); + if (remoteSet.has(name)) remote.push(name); else local.push(name); } return { local, remote }; diff --git a/apps/app/src/features/review/review-toolbar.tsx b/apps/app/src/features/review/review-toolbar.tsx index c26615304..aa2fb8b4a 100644 --- a/apps/app/src/features/review/review-toolbar.tsx +++ b/apps/app/src/features/review/review-toolbar.tsx @@ -33,7 +33,7 @@ export function ReviewToolbar({ onOtherChange: (other: string) => void; onRefresh: () => void; }) { - const refs = splitCompareRefs(branch?.branches ?? []); + const refs = splitCompareRefs(branch?.branches ?? [], branch?.remotes ?? []); const otherValue = other ?? branch?.defaultBranch ?? null; const otherItems = (branch?.branches ?? []).map((name) => ({ label: name, value: name })); diff --git a/packages/contract/src/git.ts b/packages/contract/src/git.ts index 396e39f99..d0854bc85 100644 --- a/packages/contract/src/git.ts +++ b/packages/contract/src/git.ts @@ -30,6 +30,8 @@ export const GitBranchSchema = Schema.Struct({ defaultBranch: Schema.Union([Schema.String, Schema.Null]), /** Local heads and remote-tracking refs (`main`, `origin/main`). */ branches: Schema.Array(Schema.String), + /** Remote-tracking refs only (`origin/main`). Local names may contain `/`. */ + remotes: Schema.Array(Schema.String), }); export type GitBranch = typeof GitBranchSchema.Type; diff --git a/packages/server/src/git/service.ts b/packages/server/src/git/service.ts index e149f8c87..c3b94edad 100644 --- a/packages/server/src/git/service.ts +++ b/packages/server/src/git/service.ts @@ -206,7 +206,19 @@ export const GitServiceLayer: Layer.Layer< const listRefs = (cwd: string) => raw(cwd, ["for-each-ref", "--format=%(refname)", "refs/heads", "refs/remotes"]).pipe( - Effect.map(parseRefNames), + Effect.map((output) => { + const local: string[] = []; + const remotes: string[] = []; + for (const line of output.split("\n")) { + const ref = line.trim(); + if (ref.startsWith("refs/heads/")) { + local.push(ref.slice("refs/heads/".length)); + } else if (ref.startsWith("refs/remotes/")) { + remotes.push(ref.slice("refs/remotes/".length)); + } + } + return { local, remotes, all: [...local, ...remotes] }; + }), ); const resolvePreferredCompareRef = (cwd: string) => @@ -278,7 +290,7 @@ export const GitServiceLayer: Layer.Layer< return yield* new GitRefNotFound({ ref: other ?? "" }); } const refs = yield* listRefs(cwd); - if (!refs.includes(other)) { + if (!refs.all.includes(other)) { return yield* new GitRefNotFound({ ref: other }); } const base = yield* mergeBase(cwd, other); @@ -400,8 +412,13 @@ export const GitServiceLayer: Layer.Layer< const realRoot = yield* resolveRoot(cwd); const current = yield* currentBranch(realRoot); const defaultBranch = yield* resolvePreferredCompareRef(realRoot); - const branches = yield* listRefs(realRoot); - return { current, defaultBranch, branches }; + const listed = yield* listRefs(realRoot); + return { + current, + defaultBranch, + branches: listed.all, + remotes: listed.remotes, + }; }), review: (query) => diff --git a/packages/server/test/git.test.ts b/packages/server/test/git.test.ts index ac2ef9853..9ff0a8da5 100644 --- a/packages/server/test/git.test.ts +++ b/packages/server/test/git.test.ts @@ -57,6 +57,7 @@ layer(NodePlatformLayer)("GitService", (it) => { assert.equal(branch.current, "main"); assert.equal(branch.defaultBranch, "main"); assert.ok(branch.branches.includes("main")); + assert.deepEqual(branch.remotes, []); }).pipe(Effect.provide(GitLayer)), ); @@ -71,6 +72,9 @@ layer(NodePlatformLayer)("GitService", (it) => { assert.ok(branch.branches.includes("main")); assert.ok(branch.branches.includes("origin/main")); assert.ok(branch.branches.includes("origin/HEAD")); + assert.ok(branch.remotes.includes("origin/main")); + assert.ok(branch.remotes.includes("origin/HEAD")); + assert.ok(!branch.remotes.includes("main")); }).pipe(Effect.provide(GitLayer)), ); From 02c14d7f0fd4dac8ed07a3c42cf21ff9d259b316 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 14:47:22 +0000 Subject: [PATCH 4/8] fix: open review branch select from the top so remotes stay visible alignItemWithTrigger scrolled the vs-branch menu to origin/main and clipped the Remote group. Open below the trigger so Local and origin/main are both on screen without scrolling. --- apps/app/src/features/review/review-toolbar.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/app/src/features/review/review-toolbar.tsx b/apps/app/src/features/review/review-toolbar.tsx index aa2fb8b4a..b8a340370 100644 --- a/apps/app/src/features/review/review-toolbar.tsx +++ b/apps/app/src/features/review/review-toolbar.tsx @@ -51,7 +51,7 @@ export function ReviewToolbar({ - + {REVIEW_MODE_ITEMS.map((item) => ( {item.label} @@ -71,7 +71,7 @@ export function ReviewToolbar({ - + {refs.local.length > 0 ? ( Local From 5e09dcd067624ec5210b9537cad9d791b79d5f24 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 17:01:47 +0000 Subject: [PATCH 5/8] fix: whitelist simple-git in the CLI bundle GitService is on the serve path, so tsdown inlines simple-git into the CLI. deps.onlyBundle fails closed; list simple-git and its tree so Code check can build @vibest/cli. --- packages/vibest/tsdown.config.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/vibest/tsdown.config.ts b/packages/vibest/tsdown.config.ts index 04aafb42b..2aa8bf280 100644 --- a/packages/vibest/tsdown.config.ts +++ b/packages/vibest/tsdown.config.ts @@ -6,12 +6,20 @@ export default defineConfig({ deps: { // The private server/harness/contract packages are compiled into the CLI. // Whitelist their bundled runtime dependencies so additions fail closed. + // `simple-git` (and its tree) is pulled in by GitService on the serve path. onlyBundle: [ "effect", "@effect/platform-node-shared", "@effect/platform-node", "@standardserver/shared", "@orpc/experimental-effect", + "simple-git", + /^@simple-git\//, + /^@kwsites\//, + "debug", + "ms", + "supports-color", + "has-flag", ], }, dts: false, From eef8fc5ebf15194785cd8862bcd881aea12969ae Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 17:11:33 +0000 Subject: [PATCH 6/8] fix: drop the bordered chrome from review toolbar selects Mode and branch triggers now match the project picker: transparent, no shadow, hover accent. The header already uses a ghost refresh button. --- apps/app/src/features/review/review-toolbar.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/app/src/features/review/review-toolbar.tsx b/apps/app/src/features/review/review-toolbar.tsx index b8a340370..861999488 100644 --- a/apps/app/src/features/review/review-toolbar.tsx +++ b/apps/app/src/features/review/review-toolbar.tsx @@ -14,6 +14,9 @@ import { RefreshCwIcon } from "lucide-react"; import { REVIEW_MODE_ITEMS, splitCompareRefs } from "./review-file-status"; +const GHOST_SELECT_TRIGGER = + "hover:bg-accent border-transparent bg-transparent shadow-none before:hidden dark:bg-transparent"; + export function ReviewToolbar({ mode, other, @@ -48,7 +51,11 @@ export function ReviewToolbar({ }} value={mode} > - + @@ -68,7 +75,11 @@ export function ReviewToolbar({ }} value={otherValue} > - + From bcb2b02e285fe5ccc5ff6533cd15d2f7404098b0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 17 Aug 2026 17:28:47 +0000 Subject: [PATCH 7/8] fix: use a chevron-down icon on review toolbar selects Coss SelectTrigger hard-codes ChevronsUpDown. Render the same ghost Button chrome as the harness picker so compare mode and vs-branch use ChevronDown. --- .../src/features/review/review-toolbar.tsx | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/apps/app/src/features/review/review-toolbar.tsx b/apps/app/src/features/review/review-toolbar.tsx index 861999488..7227e88c6 100644 --- a/apps/app/src/features/review/review-toolbar.tsx +++ b/apps/app/src/features/review/review-toolbar.tsx @@ -10,12 +10,36 @@ import { SelectValue, } from "@vibest/ui/components/select"; import { cn } from "@vibest/ui/lib/utils"; -import { RefreshCwIcon } from "lucide-react"; +import { ChevronDownIcon, RefreshCwIcon } from "lucide-react"; +import type { ComponentProps } from "react"; import { REVIEW_MODE_ITEMS, splitCompareRefs } from "./review-file-status"; -const GHOST_SELECT_TRIGGER = - "hover:bg-accent border-transparent bg-transparent shadow-none before:hidden dark:bg-transparent"; +function GhostSelectTrigger({ + className, + placeholder, + ...props +}: Omit, "render"> & { + placeholder?: string; +}) { + // Drop SelectTrigger's field chrome and its hard-coded up/down icon. + return ( + ( + + )} + /> + ); +} export function ReviewToolbar({ mode, @@ -51,13 +75,7 @@ export function ReviewToolbar({ }} value={mode} > - - - + {REVIEW_MODE_ITEMS.map((item) => ( @@ -75,13 +93,11 @@ export function ReviewToolbar({ }} value={otherValue} > - - - + className="min-w-0 flex-1" + placeholder="Select a branch" + /> {refs.local.length > 0 ? ( From 21551aaaecddea2c0ee7ea8e6ee81bda2176a286 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 3 Sep 2026 13:15:43 +0000 Subject: [PATCH 8/8] fix: stop server tests timing out under CI load @effect/vitest layer({ timeout }) only covers hooks, so spawn tests still hit vitest's 5s default while turbo runs build/test/typecheck together. Raise the package testTimeout to 30s and poll for stop tombstones instead of sleeping 100ms. --- packages/server/test/daemon/launcher.test.ts | 16 ++++++++++++++-- packages/server/vitest.config.ts | 5 +++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/server/test/daemon/launcher.test.ts b/packages/server/test/daemon/launcher.test.ts index 87a79d18c..3d3e40275 100644 --- a/packages/server/test/daemon/launcher.test.ts +++ b/packages/server/test/daemon/launcher.test.ts @@ -38,7 +38,8 @@ const resolve = ( // `excludeTestServices` because the launcher polls a real daemon's health on a // real clock: under the default TestClock its retry schedule never advances. -// The timeout covers a spawn + readiness handshake, not a unit assertion. +// `timeout` here is the hook budget (layer build / daemon teardown). The +// per-test spawn + readiness budget lives in `vitest.config.ts`. layer(NodeServices.layer, { excludeTestServices: true, timeout: "30 seconds" })( "resolveOrSpawnDaemon", (it) => { @@ -269,7 +270,18 @@ layer(NodeServices.layer, { excludeTestServices: true, timeout: "30 seconds" })( ); const stopFiber = yield* Effect.forkScoped(stopDaemon(daemonDir, legacyDaemonDir)); - yield* Effect.sleep("100 millis"); + // `stopDaemon` writes tombstones before waiting on the locks this test + // holds. Poll instead of a fixed sleep: under load the fiber may not + // have run within 100ms. + yield* Effect.sleep("10 millis").pipe( + Effect.repeat({ + while: () => + Effect.gen(function* () { + return !(yield* hasTombstone(legacyDaemonDir)) || !(yield* hasTombstone(daemonDir)); + }), + }), + Effect.timeout("5 seconds"), + ); assert.equal(yield* hasTombstone(legacyDaemonDir), true); assert.equal(yield* hasTombstone(daemonDir), true); // Simulate a compatible explicit launcher clearing the early stop diff --git a/packages/server/vitest.config.ts b/packages/server/vitest.config.ts index 4e19108dc..c862ce8d0 100644 --- a/packages/server/vitest.config.ts +++ b/packages/server/vitest.config.ts @@ -3,6 +3,11 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { environment: "node", + // Spawn + readiness tests (daemon launcher, harness transports) share this + // runner with turbo's parallel build/typecheck. Vitest's 5s default is too + // tight under that load, and `@effect/vitest` `layer({ timeout })` only + // covers hooks — not `it.effect`. + testTimeout: 30_000, typecheck: { enabled: true, tsconfig: "./tsconfig.json",