diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index a233567b5f..24b45a706d 100644 --- a/packages/app/e2e/browser/changes-pane.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -262,6 +262,93 @@ test.afterEach(async () => { } }); +test("streamed Changes shows the tree before completion and applies a one-file refresh", async ({ + page, +}, testInfo) => { + test.setTimeout(120_000); + const workspace = await createLargeStreamedWorkspace(); + const stream = await holdDiffCompletion(page); + await openWorkspaceChangesSurface(page, workspace, 90_000); + await expectAnnouncedTreeAndLoader(page); + await page.screenshot({ path: testInfo.outputPath("tree-loading.png") }); + stream.finish(); + await expect.poll(() => stream.completed, { timeout: 60_000 }).toBeGreaterThan(0); + await expect(page.getByRole("progressbar", { name: "Loading..." })).toHaveCount(0); + const connections = stream.connections; + const panel = page.getByTestId("working-diff-panel").filter({ visible: true }); + await expect(panel.getByTestId("diff-file-0")).toHaveAccessibleName("src/file-0000.ts, +90, -1"); + await addOneLineToFirstFile(workspace); + await expect.poll(() => stream.changedBodies.at(-1), { timeout: 30_000 }).toBe(1); + await expect(panel.getByTestId("diff-file-0")).toHaveAccessibleName("src/file-0000.ts, +91, -1"); + await expect(page.getByRole("progressbar", { name: "Loading..." })).toHaveCount(0); + expect(stream.connections).toBe(connections); + await page.screenshot({ path: testInfo.outputPath("refreshed.png") }); +}); + +async function createLargeStreamedWorkspace(): Promise { + const workspace = await createWorkspaceWithManyTinyDiffs(252); + await Promise.all( + Array.from({ length: 252 }, (_, file) => + writeFile( + path.join(workspace.repoPath, `src/file-${String(file).padStart(4, "0")}.ts`), + Array.from( + { length: 90 }, + (_unused, line) => + `export const fn${line} = (param: string) => ({ key: param, value: ${file} });`, + ).join("\n") + "\n", + ), + ), + ); + return workspace; +} +async function addOneLineToFirstFile(workspace: DirtyWorkspace): Promise { + const file = path.join(workspace.repoPath, "src/file-0000.ts"); + await writeFile(file, (await readFile(file, "utf8")) + "export const refreshed = true;\n"); +} +async function holdDiffCompletion(page: Page) { + const { decodeDiffFrame, DiffOpcode } = await import("@getpaseo/protocol/binary-frames/index"); + let holding = true; + let connections = 0; + let completed = 0; + const completions: Array<() => void> = []; + const changedBodies: number[] = []; + await page.routeWebSocket(daemonWsRoutePattern(), (browserSocket) => { + connections++; + const serverSocket = browserSocket.connectToServer(); + browserSocket.onMessage((message) => serverSocket.send(message)); + serverSocket.onMessage((message) => { + if (typeof message !== "string" && message[0] === DiffOpcode.Begin) { + const frame = decodeDiffFrame(message); + if (frame.opcode === DiffOpcode.Begin) + changedBodies.push(frame.entries.filter((entry) => entry.body).length); + } + if (typeof message !== "string" && message[0] === DiffOpcode.End) completed++; + if (holding && typeof message !== "string" && message[0] === DiffOpcode.End) { + completions.push(() => browserSocket.send(message)); + return; + } + browserSocket.send(message); + }); + }); + return { + changedBodies, + get connections() { + return connections; + }, + get completed() { + return completed; + }, + finish: () => { + holding = false; + completions.splice(0).forEach((send) => send()); + }, + }; +} +async function expectAnnouncedTreeAndLoader(page: Page): Promise { + await expect(changesTree(page).first()).toBeVisible(); + await expect(page.getByRole("progressbar", { name: "Loading..." }).first()).toBeVisible(); +} + test("Changes opens All changes for a clean checkout", async ({ page }) => { const workspace = await createWorkspaceWithCommittedDiff(); @@ -490,6 +577,35 @@ test("an empty Changes comparison links to the populated comparison", async ({ p await expect(panel.getByTestId("diff-file-0")).toHaveAccessibleName("committed-only.ts, +1, -0"); }); +test("legacy diff hosts keep All, Committed and Uncommitted comparisons", async ({ page }) => { + await useLegacyDiffHost(page); + const workspace = await createWorkspaceWithScrollableComparisons(); + await openWorkspaceChangesSurface(page, workspace); + await expect(changesTree(page).getByText("committed", { exact: true })).toBeVisible(); + await expect(changesTree(page).getByText("uncommitted", { exact: true })).toBeVisible(); + await selectChangesComparison(page, "Committed"); + await expectWorkingComparisonFiles(page, "committed"); + await selectChangesComparison(page, "Uncommitted"); + await expectWorkingComparisonFiles(page, "uncommitted"); +}); +async function useLegacyDiffHost(page: Page): Promise { + await page.routeWebSocket(daemonWsRoutePattern(), (browserSocket) => { + const serverSocket = browserSocket.connectToServer(); + browserSocket.onMessage((message) => serverSocket.send(message)); + serverSocket.onMessage((message) => { + if (typeof message === "string") { + const envelope = JSON.parse(message); + if (envelope.message?.payload?.status === "server_info") { + delete envelope.message.payload.features.diffSubscription; + browserSocket.send(JSON.stringify(envelope)); + return; + } + } + browserSocket.send(message); + }); + }); +} + test("Changes comparison controls the working diff and tree selection focuses its file", async ({ page, }) => { diff --git a/packages/app/src/data/push-router.test.ts b/packages/app/src/data/push-router.test.ts index 6599c28467..432d8b8224 100644 --- a/packages/app/src/data/push-router.test.ts +++ b/packages/app/src/data/push-router.test.ts @@ -1,4 +1,4 @@ -import type { OwnedSubscription, SubscriptionObserver } from "@getpaseo/client"; +import type { DiffState, OwnedSubscription, SubscriptionObserver } from "@getpaseo/client"; import { QueryClient, QueryObserver, skipToken } from "@tanstack/react-query"; import { describe, expect, it } from "vitest"; import type { MutableDaemonConfig, SessionOutboundMessage } from "@getpaseo/protocol/messages"; @@ -144,10 +144,21 @@ function createFakeClient(config: { rejectCheckoutDiffSubscribe?: boolean } = {} ["status", "providers_snapshot_update"], () => {}, ), - observeCheckoutDiff(cwd, compare) { + observeDiff(input) { + const cwd = input.cwd; + let mode: "uncommitted" | "base" | "all" = "uncommitted"; + if (input.head.kind === "rev") mode = "base"; + else if (input.mergeBase) mode = "all"; + const compare = { + mode, + ...(input.base.kind === "rev" && input.base.rev !== "HEAD" + ? { baseRef: input.base.rev } + : {}), + ignoreWhitespace: input.ignoreWhitespace, + }; const subscriptionId = `server-diff-${subscribeCheckoutDiffCalls.length + 1}`; subscribeCheckoutDiffCalls.push({ cwd, compare, subscriptionId }); - const handle = observe( + const handle = observe( { subscriptionId, cwd, files: [], error: null, requestId: "subscribe-checkout-diff" }, ["checkout_diff_update", "subscribe_checkout_diff_response"], () => unsubscribeCheckoutDiffCalls.push(subscriptionId), @@ -155,9 +166,43 @@ function createFakeClient(config: { rejectCheckoutDiffSubscribe?: boolean } = {} "subscriptionId" in message.payload && message.payload.subscriptionId === subscriptionId, ); - if (!config.rejectCheckoutDiffSubscribe) return handle; - return { + const modern: OwnedSubscription = { ...handle, + ready: Promise.resolve({ + phase: "ready", + revision: 1, + resolved: { baseSha: "", headSha: "", mergeBaseSha: "" }, + files: [], + diffTooLarge: false, + subscriptionId, + }), + subscribe: (observer) => + handle.subscribe({ + snapshot: (payload) => + observer.snapshot({ + phase: "ready", + revision: 1, + resolved: { baseSha: "", headSha: "", mergeBaseSha: "" }, + files: payload.files, + diffTooLarge: payload.diffTooLarge === true, + subscriptionId, + }), + update: (message) => { + if (message.type === "checkout_diff_update") + observer.snapshot({ + phase: "ready", + revision: 2, + resolved: { baseSha: "", headSha: "", mergeBaseSha: "" }, + files: message.payload.files, + diffTooLarge: message.payload.diffTooLarge === true, + subscriptionId, + }); + }, + }), + }; + if (!config.rejectCheckoutDiffSubscribe) return modern; + return { + ...modern, subscribe: (observer) => { queueMicrotask(() => observer.error?.(new Error("subscribe failed"))); return () => {}; @@ -328,7 +373,7 @@ describe("server data push router", () => { const first = publish(files, "first"); const same = publish(structuredClone(files), "reopen"); expect(same.files).toBe(first.files); - expect(same.requestId).toBe("reopen"); + expect(same.requestId).toBe("diff:1:ready"); const changed = structuredClone(files); changed[0]!.hunks[0]!.lines[0]!.tokens![0]!.style = "variable"; const next = publish(changed, "edit"); @@ -430,7 +475,7 @@ describe("server data push router", () => { files: [], error: null, diffTooLarge: true, - requestId: "diff-1", + requestId: "diff:1:ready", }); fake.emit({ @@ -449,7 +494,7 @@ describe("server data push router", () => { files: [], error: null, diffTooLarge: true, - requestId: `subscription:${serverSubscriptionId}`, + requestId: "diff:2:ready", }); unsubscribeObserver(); diff --git a/packages/app/src/data/push-router.ts b/packages/app/src/data/push-router.ts index cc32f8f8e3..5e51ded0d4 100644 --- a/packages/app/src/data/push-router.ts +++ b/packages/app/src/data/push-router.ts @@ -1,3 +1,5 @@ +import { checkoutDiffSnapshot, type CheckoutDiffSnapshot } from "@/git/diff-snapshot"; +import type { DiffState } from "@getpaseo/client"; import type { OwnedSubscription } from "@getpaseo/client"; import type { ProviderSnapshotEntry } from "@getpaseo/protocol/agent-types"; import type { Query, QueryCacheNotifyEvent, QueryClient, QueryKey } from "@tanstack/react-query"; @@ -23,14 +25,9 @@ type ProvidersSnapshotUpdateMessage = Extract< SessionOutboundMessage, { type: "providers_snapshot_update" } >; -type SubscribeCheckoutDiffResponseMessage = Extract< - SessionOutboundMessage, - { type: "subscribe_checkout_diff_response" } ->; type StatusMessage = Extract; type TerminalsChangedMessage = Extract; -type CheckoutDiffResponsePayload = SubscribeCheckoutDiffResponseMessage["payload"]; -type CheckoutDiffCachePayload = Omit; +type CheckoutDiffCachePayload = CheckoutDiffSnapshot; type ListTerminalsPayload = ListTerminalsResponse["payload"]; interface CheckoutDiffCompare { @@ -49,7 +46,7 @@ interface CheckoutDiffRoute { } interface CheckoutDiffRegistration extends CheckoutDiffRoute { - subscription: OwnedSubscription; + subscription: OwnedSubscription; } interface WorkspaceTerminalsRoute { @@ -75,7 +72,7 @@ export type ProvidersSnapshotUpdate = ProvidersSnapshotUpdateMessage; interface ServerDataPushClient { getProvidersSnapshot: import("@getpaseo/client/internal/daemon-client").DaemonClient["getProvidersSnapshot"]; observeEvents: import("@getpaseo/client/internal/daemon-client").DaemonClient["observeEvents"]; - observeCheckoutDiff: import("@getpaseo/client/internal/daemon-client").DaemonClient["observeCheckoutDiff"]; + observeDiff: import("@getpaseo/client/internal/daemon-client").DaemonClient["observeDiff"]; observeTerminals: import("@getpaseo/client/internal/daemon-client").DaemonClient["observeTerminals"]; } @@ -339,34 +336,34 @@ function reconcileCheckoutDiffSubscriptions(input: { } for (const [key, desired] of input.desired) { if (input.active.has(key)) continue; - const subscription = input.client.observeCheckoutDiff(desired.cwd, desired.compare); + const { mode, baseRef, ignoreWhitespace } = desired.compare; + const revision = mode === "uncommitted" ? "HEAD" : baseRef; + if (!revision) continue; + const subscription = input.client.observeDiff({ + cwd: desired.cwd, + base: { kind: "rev", rev: revision }, + head: mode === "base" ? { kind: "rev", rev: "HEAD" } : { kind: "worktree" }, + mergeBase: mode !== "uncommitted", + ignoreWhitespace, + }); const registration = { ...desired, subscription }; input.active.set(key, registration); - const apply = ( - payload: Omit & { requestId?: string }, - ) => { - if (input.active.get(key) !== registration) return; - const { subscriptionId, ...snapshot } = payload; - setCheckoutDiffPayload({ - activeCheckoutDiffSubscriptions: input.active, - queryClient: input.queryClient, - serverId: input.serverId, - subscriptionId: key, - payload: { - ...snapshot, - files: orderCheckoutDiffFiles(payload.files), - requestId: payload.requestId ?? `subscription:${subscriptionId}`, - }, - }); - }; subscription.subscribe({ - snapshot: apply, - update: (message) => { - if (message.type === "checkout_diff_update") apply(message.payload); + snapshot: (state) => { + if (input.active.get(key) !== registration) return; + const snapshot = checkoutDiffSnapshot(desired.cwd, state); + setCheckoutDiffPayload({ + activeCheckoutDiffSubscriptions: input.active, + queryClient: input.queryClient, + serverId: input.serverId, + subscriptionId: key, + payload: { ...snapshot, files: orderCheckoutDiffFiles(snapshot.files) }, + }); }, + update: () => {}, error: (error) => { if (input.active.get(key) === registration) input.active.delete(key); - console.error("[server-data] observeCheckoutDiff failed", { + console.error("[server-data] observeDiff failed", { serverId: input.serverId, cwd: desired.cwd, error, diff --git a/packages/app/src/git/changed-files-tree.tsx b/packages/app/src/git/changed-files-tree.tsx index 0b88d33e96..f020cfa101 100644 --- a/packages/app/src/git/changed-files-tree.tsx +++ b/packages/app/src/git/changed-files-tree.tsx @@ -13,10 +13,10 @@ import { type DiffTreeRow, } from "@/git/diff-tree"; import { FileHeader } from "@/git/file-header"; -import type { ParsedDiffFile } from "@/git/use-diff-query"; +import type { DiffFileHeader } from "@/git/diff-snapshot"; export interface ChangedFilesTreeProps { - files: ParsedDiffFile[]; + files: DiffFileHeader[]; mode: WorkingDiffMode; onSelectFile: (path: string) => void; collapsedFolderPaths: string[]; diff --git a/packages/app/src/git/diff-order.ts b/packages/app/src/git/diff-order.ts index d57bb0ef29..0ad572866f 100644 --- a/packages/app/src/git/diff-order.ts +++ b/packages/app/src/git/diff-order.ts @@ -13,6 +13,6 @@ export function orderCheckoutDiffFiles(files: ParsedDiffFile[]): ParsedDiffFile[ // chains, so it is skipped; the collapsed set is empty because the flat list // always carries every file, whatever the rail has collapsed. return flattenDiffTree(buildDiffTree(files), new Set()).flatMap((row) => - row.kind === "file" ? [row.file] : [], + row.kind === "file" ? [files[row.fileIndex]] : [], ); } diff --git a/packages/app/src/git/diff-pane.tsx b/packages/app/src/git/diff-pane.tsx index d4a7d43ea8..8fe6a909e8 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -1,3 +1,4 @@ +import type { DiffFileHeader } from "@/git/diff-snapshot"; import { useSafeAreaInsets } from "react-native-safe-area-context"; import { floatingActionsClearance } from "@/components/ui/floating-action-button"; import { useState, useCallback, useMemo, type ReactElement, type ReactNode } from "react"; @@ -1095,6 +1096,7 @@ interface DiffBodyContentProps { emptyMessage: string; emptyAction: ChangesEmptyAction | null; children: ReactElement; + loadingLabel: string; checkingRepositoryLabel: string; notRepositoryLabel: string; } @@ -1111,11 +1113,16 @@ function DiffBodyContent({ emptyAction, children, checkingRepositoryLabel, + loadingLabel, notRepositoryLabel, }: DiffBodyContentProps) { if (isStatusLoading) { return ( - + {checkingRepositoryLabel} @@ -1137,7 +1144,11 @@ function DiffBodyContent({ } if (isDiffLoading) { return ( - + ); @@ -1321,7 +1332,7 @@ function ChangesTreeRail({ }: { shown: boolean; children: ReactElement; - files: ParsedDiffFile[]; + files: DiffFileHeader[]; mode: WorkingDiffMode; onSelectFile: (path: string) => void; treeWidth?: number; @@ -1361,7 +1372,7 @@ function ChangesBody({ children: ReactElement; desktopTreeVisible: boolean; isMobile: boolean; - files: ParsedDiffFile[]; + files: DiffFileHeader[]; mode: WorkingDiffMode; onSelectFile: (path: string) => void; treeWidth?: number; @@ -1586,6 +1597,7 @@ export function ChangesSurface({ selectBase: handleSelectBase, selectAll: handleSelectAll, files, + treeFiles, diffPayloadError, diffTooLarge, isDiffLoading, @@ -1835,6 +1847,7 @@ export function ChangesSurface({ hasChanges={hasChanges} emptyMessage={emptyMessage} emptyAction={emptyAction} + loadingLabel={t("common.loading")} checkingRepositoryLabel={t("workspace.git.diff.checkingRepository")} notRepositoryLabel={t("workspace.git.diff.notRepository")} > @@ -1865,7 +1878,7 @@ export function ChangesSurface({ presentation={presentation} desktopTreeVisible={desktopTreeVisible} isMobile={isMobile} - files={files} + files={treeFiles} mode={workingMode} onSelectFile={handleSelectTreeFile} treeWidth={instanceState.treeWidth} diff --git a/packages/app/src/git/diff-sharing.ts b/packages/app/src/git/diff-sharing.ts index 6bdac10ddf..d426960d9f 100644 --- a/packages/app/src/git/diff-sharing.ts +++ b/packages/app/src/git/diff-sharing.ts @@ -1,7 +1,8 @@ import { replaceEqualDeep } from "@tanstack/react-query"; -import type { ParsedDiffFile, SubscribeCheckoutDiffResponse } from "@getpaseo/protocol/messages"; +import type { CheckoutDiffSnapshot } from "./diff-snapshot"; +import type { ParsedDiffFile } from "@getpaseo/protocol/messages"; -type DiffPayload = Omit; +type DiffPayload = CheckoutDiffSnapshot; /** File identity owns text-model reuse. Never reconstruct the hunk/token graph. */ export function retainDiffFiles( @@ -30,6 +31,7 @@ export function shareCheckoutDiff(previous: unknown, incoming: unknown): DiffPay error === old.error && old.cwd === next.cwd && old.requestId === next.requestId && + old.entries === next.entries && old.diffTooLarge === next.diffTooLarge ? old : { ...next, files, error }; diff --git a/packages/app/src/git/diff-snapshot.ts b/packages/app/src/git/diff-snapshot.ts new file mode 100644 index 0000000000..25f073b4ef --- /dev/null +++ b/packages/app/src/git/diff-snapshot.ts @@ -0,0 +1,60 @@ +import type { DiffState, DiffEntry } from "@getpaseo/client"; +import type { DiffError, ParsedDiffFile } from "@getpaseo/protocol/messages"; +export type DiffFileHeader = Omit; +export interface CheckoutDiffSnapshot { + requestId: string; + cwd: string; + files: ParsedDiffFile[]; + entries?: DiffEntry[]; + error: DiffError | null; + diffTooLarge?: boolean; +} +export function checkoutDiffSnapshot(cwd: string, state: DiffState): CheckoutDiffSnapshot { + const common = { cwd, requestId: `diff:${state.revision}:${state.phase}` }; + if (state.phase === "loading") + return { ...common, files: [], entries: state.entries, error: null }; + if (state.phase === "error") return { ...common, files: [], error: state.error }; + return { ...common, files: state.files, error: null, diffTooLarge: state.diffTooLarge }; +} +export function diffEntryHeaders(entries: DiffEntry[]): DiffFileHeader[] { + return entries.map((entry) => ({ + path: entry.path, + ...(entry.oldPath === null ? {} : { oldPath: entry.oldPath }), + isNew: entry.change === "added", + isDeleted: entry.change === "deleted", + additions: entry.additions, + deletions: entry.deletions, + status: entry.content === "binary" ? "binary" : "ok", + })); +} +export interface CheckoutDiffResult { + files: ParsedDiffFile[]; + entries: DiffEntry[] | null; + payloadError: CheckoutDiffSnapshot["error"]; + diffTooLarge: boolean; + isLoading: boolean; + isFetching: boolean; + isError: boolean; + error: null; +} + +/** + * The result derives from the payload alone. Until a payload arrives there is no + * diff to describe, so the boundary reports loading rather than an empty diff — + * whether the query is idle (an inactive retained panel), in flight, or the host + * is disconnected. Reporting `files: []` as settled made callers render "+0 -0" + * and "No changes" for a diff nobody has fetched yet. + */ +export function deriveCheckoutDiffResult(payload: CheckoutDiffSnapshot | null): CheckoutDiffResult { + const payloadError = payload?.error ?? null; + return { + files: payload?.files ?? [], + entries: payload?.entries ?? null, + payloadError, + diffTooLarge: payload?.diffTooLarge === true, + isLoading: payload === null || payload.entries !== undefined, + isFetching: false, + isError: Boolean(payloadError), + error: null, + }; +} diff --git a/packages/app/src/git/diff-tree.ts b/packages/app/src/git/diff-tree.ts index 49709cd5e2..c3130b1cae 100644 --- a/packages/app/src/git/diff-tree.ts +++ b/packages/app/src/git/diff-tree.ts @@ -1,6 +1,6 @@ -import type { ParsedDiffFile } from "@/git/use-diff-query"; +import type { DiffFileHeader } from "@/git/diff-snapshot"; -// Builds a directory hierarchy from the `ParsedDiffFile[]` the Changes view +// Builds a directory hierarchy from the `DiffFileHeader[]` the Changes view // renders. The tree renders on every form factor, consistent with the Files // explorer. // @@ -17,7 +17,7 @@ import type { ParsedDiffFile } from "@/git/use-diff-query"; export interface DiffTreeFileNode { kind: "file"; - file: ParsedDiffFile; + file: DiffFileHeader; fileIndex: number; /** basename, e.g. "diff-pane.tsx" */ name: string; @@ -47,7 +47,7 @@ export interface DiffTreeFolderRow { export interface DiffTreeFileRow { kind: "file"; - file: ParsedDiffFile; + file: DiffFileHeader; fileIndex: number; depth: number; } @@ -73,7 +73,7 @@ function sortTree(node: DiffTreeDirNode): void { } /** Build the (uncompressed) directory tree. Returns the virtual root (dirPath ""). */ -export function buildDiffTree(files: ParsedDiffFile[]): DiffTreeDirNode { +export function buildDiffTree(files: DiffFileHeader[]): DiffTreeDirNode { const root: DiffTreeDirNode = { kind: "dir", dirPath: "", name: "", children: [] }; const dirByPath = new Map([["", root]]); diff --git a/packages/app/src/git/file-header.tsx b/packages/app/src/git/file-header.tsx index d19d58cbf2..39427a612b 100644 --- a/packages/app/src/git/file-header.tsx +++ b/packages/app/src/git/file-header.tsx @@ -28,11 +28,11 @@ import { fileNameForPath, formatDiffCount, } from "@/git/file-header-presentation"; -import type { ParsedDiffFile } from "@/git/use-diff-query"; +import type { DiffFileHeader } from "@/git/diff-snapshot"; import { inlineUnistylesStyle } from "@/styles/unistyles-inline-style"; export interface FileHeaderProps { - file: ParsedDiffFile; + file: DiffFileHeader; workspaceFileDragScope?: { serverId: string; workspaceId: string }; bodyVisible: boolean; showsBodyState?: boolean; @@ -73,7 +73,7 @@ function expandedAriaValue(showsBodyState: boolean, bodyVisible: boolean): boole return showsBodyState ? bodyVisible : undefined; } -function fileHeaderAccessibilityLabel(canvasRendered: boolean, file: ParsedDiffFile) { +function fileHeaderAccessibilityLabel(canvasRendered: boolean, file: DiffFileHeader) { if (!canvasRendered) return undefined; return `${file.path}, +${formatDiffCount(file.additions)}, -${formatDiffCount(file.deletions)}`; } @@ -130,7 +130,7 @@ function fileHeaderNameStyle(showsBodyState: boolean, isHovered: boolean) { ]; } -function fileChange(file: ParsedDiffFile): "added" | "deleted" | "modified" { +function fileChange(file: DiffFileHeader): "added" | "deleted" | "modified" { return diffFileChangeKind(file); } diff --git a/packages/app/src/git/use-diff-query.test.ts b/packages/app/src/git/use-diff-query.test.ts index 94b18a7d16..1d5f644a12 100644 --- a/packages/app/src/git/use-diff-query.test.ts +++ b/packages/app/src/git/use-diff-query.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { ParsedDiffFile } from "@getpaseo/protocol/messages"; -import { deriveCheckoutDiffResult } from "./use-diff-query"; +import { deriveCheckoutDiffResult } from "./diff-snapshot"; function createParsedDiffFile(path: string): ParsedDiffFile { return { @@ -59,6 +59,7 @@ describe("deriveCheckoutDiffResult", () => { expect(result).toEqual({ files: [], + entries: null, payloadError: error, diffTooLarge: true, isLoading: false, @@ -68,3 +69,20 @@ describe("deriveCheckoutDiffResult", () => { }); }); }); + +it("shows the announced tree while file bodies are still loading", () => { + const entries = [ + { + path: "src/app.ts", + oldPath: null, + change: "modified", + content: "text", + additions: 3, + deletions: 1, + }, + ] as const; + const result = deriveCheckoutDiffResult({ ...createPayload([]), entries: [...entries] }); + expect(result.isLoading).toBe(true); + expect(result.entries).toEqual(entries); + expect(result.files).toEqual([]); +}); diff --git a/packages/app/src/git/use-diff-query.ts b/packages/app/src/git/use-diff-query.ts index ebfcd4c354..7f1d3ab521 100644 --- a/packages/app/src/git/use-diff-query.ts +++ b/packages/app/src/git/use-diff-query.ts @@ -5,7 +5,9 @@ import { useRetainedPanelActive } from "@/components/retained-panel"; import { useReplicaQuery } from "@/data/query"; import { checkoutDiffPushRoute } from "@/data/push-router"; import { useHostRuntimeIsConnected } from "@/runtime/host-runtime"; -import type { ParsedDiffFile, SubscribeCheckoutDiffResponse } from "@getpaseo/protocol/messages"; +import { deriveCheckoutDiffResult, type CheckoutDiffSnapshot } from "./diff-snapshot"; +export { deriveCheckoutDiffResult } from "./diff-snapshot"; +import type { ParsedDiffFile } from "@getpaseo/protocol/messages"; import { checkoutDiffQueryKey } from "@/git/query-keys"; interface UseCheckoutDiffQueryOptions { @@ -18,7 +20,7 @@ interface UseCheckoutDiffQueryOptions { queryScope?: string; } -type CheckoutDiffQueryPayload = Omit; +type CheckoutDiffQueryPayload = CheckoutDiffSnapshot; // Re-export the canonical protocol type so all consumers share one definition. export type { ParsedDiffFile }; @@ -108,35 +110,3 @@ export function useCheckoutDiffQuery({ : (query.data ?? null), ); } - -export interface CheckoutDiffResult { - files: ParsedDiffFile[]; - payloadError: CheckoutDiffQueryPayload["error"]; - diffTooLarge: boolean; - isLoading: boolean; - isFetching: boolean; - isError: boolean; - error: null; -} - -/** - * The result derives from the payload alone. Until a payload arrives there is no - * diff to describe, so the boundary reports loading rather than an empty diff — - * whether the query is idle (an inactive retained panel), in flight, or the host - * is disconnected. Reporting `files: []` as settled made callers render "+0 -0" - * and "No changes" for a diff nobody has fetched yet. - */ -export function deriveCheckoutDiffResult( - payload: CheckoutDiffQueryPayload | null, -): CheckoutDiffResult { - const payloadError = payload?.error ?? null; - return { - files: payload?.files ?? [], - payloadError, - diffTooLarge: payload?.diffTooLarge === true, - isLoading: payload === null, - isFetching: false, - isError: Boolean(payloadError), - error: null, - }; -} diff --git a/packages/app/src/git/use-working-diff.ts b/packages/app/src/git/use-working-diff.ts index f835d44f53..9f32ce8819 100644 --- a/packages/app/src/git/use-working-diff.ts +++ b/packages/app/src/git/use-working-diff.ts @@ -9,6 +9,8 @@ import { useReviewAttachmentSnapshot, useReviewFeedback, } from "@/review"; +import { useSessionStore } from "@/stores/session-store"; +import { diffEntryHeaders } from "./diff-snapshot"; import { useCheckoutDiffQuery } from "@/git/use-diff-query"; import { useCheckoutStatusQuery } from "@/git/use-status-query"; import { useWorkingDiffComparison } from "@/git/working-diff-comparison"; @@ -44,7 +46,11 @@ export function useWorkingDiff({ const statusErrorMessage = status?.error?.message ?? (isStatusError && statusError instanceof Error ? statusError.message : null); - const baseRef = gitStatus?.baseRef ?? undefined; + const modernDiff = useSessionStore( + (state) => state.sessions[serverId]?.serverInfo?.features?.diffSubscription === true, + ); + // COMPAT(diffSubscription): added in v0.15.0, remove after 2027-05-06. + const baseRef = (modernDiff ? gitStatus?.comparisonBaseRef : gitStatus?.baseRef) ?? undefined; const currentBranchName = gitStatus?.currentBranch && gitStatus.currentBranch !== "HEAD" ? gitStatus.currentBranch : null; @@ -59,6 +65,7 @@ export function useWorkingDiff({ const { files, + entries, payloadError: diffPayloadError, diffTooLarge, isLoading: isDiffLoading, @@ -71,6 +78,7 @@ export function useWorkingDiff({ enabled: enabled && isGit, queryScope, }); + const treeFiles = useMemo(() => (entries ? diffEntryHeaders(entries) : files), [entries, files]); const reviewDraftKey = useMemo( () => buildReviewDraftKey({ @@ -112,6 +120,7 @@ export function useWorkingDiff({ selectBase, selectAll, files, + treeFiles, diffPayloadError, diffTooLarge, isDiffLoading, diff --git a/packages/server/src/utils/checkout-git.commits.test.ts b/packages/server/src/utils/checkout-git.commits.test.ts index 1f82fe73b0..dc57333cee 100644 --- a/packages/server/src/utils/checkout-git.commits.test.ts +++ b/packages/server/src/utils/checkout-git.commits.test.ts @@ -424,3 +424,17 @@ it("compares tag-based worktrees in status, history and legacy diff modes", asyn const all = await getCheckoutDiff(repoDir, { mode: "all", baseRef: "refs/tags/v1" }); expect(all.diff).toContain("+edited"); }); + +it("announces a usable comparison base while checked out on the base branch", async () => { + const cwd = makeTempDir(); + git(["init", "-b", "main"], cwd); + writeFileSync(join(cwd, "a.txt"), "before\n"); + git(["add", "."], cwd); + commit(cwd, "base"); + writeFileSync(join(cwd, "a.txt"), "after\n"); + const status = await getCheckoutStatus(cwd); + expect(status).toMatchObject({ isGit: true, comparisonBaseRef: "refs/heads/main" }); + if (!status.isGit || !status.comparisonBaseRef) throw new Error("Missing comparison base"); + const all = await getCheckoutDiff(cwd, { mode: "all", baseRef: status.comparisonBaseRef }); + expect(all.diff).toContain("+after"); +}); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 50d312a757..a9109ba6d1 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -1786,6 +1786,16 @@ async function resolveBestComparisonBaseRef( return resolveCommitComparisonRef(cwd, baseRef, context); } +async function resolveQualifiedComparisonBaseRef( + cwd: string, + baseRef: string, + context: CheckoutContext, +): Promise { + const comparison = context.facts?.isGit ? context.facts.comparisonBaseRef : null; + const ref = comparison ?? (await resolveBestComparisonBaseRef(cwd, baseRef, context)); + return resolveCommitComparisonRef(cwd, ref, context); +} + async function resolveMostAheadBaseRef(cwd: string, baseRef: string): Promise { if (isQualifiedBranchRef(baseRef)) { if (await doesGitRefExist(cwd, baseRef)) { @@ -2404,6 +2414,10 @@ export async function getCheckoutStatus( const baseRef = facts.resolvedBaseRef; const mainRepoRoot = facts.mainRepoRoot; const factsContext = { ...context, facts }; + // Ref-derived counts skip the current base branch; the diff consumer still needs its ref. + const comparisonBaseRef = baseRef + ? await resolveQualifiedComparisonBaseRef(cwd, baseRef, factsContext).catch(() => null) + : null; const aheadBehind = baseRef && currentBranch ? await getAheadBehind(cwd, baseRef, currentBranch, factsContext) @@ -2426,7 +2440,7 @@ export async function getCheckoutStatus( isDirty, baseRef: displayBaseRef ?? baseRef, baseRefExact: baseRef, - comparisonBaseRef: facts.comparisonBaseRef, + comparisonBaseRef, aheadBehind, upstreamRef, aheadOfOrigin, @@ -2446,7 +2460,7 @@ export async function getCheckoutStatus( isDirty, baseRef: displayBaseRef, baseRefExact: baseRef, - comparisonBaseRef: facts.comparisonBaseRef, + comparisonBaseRef, aheadBehind, upstreamRef, aheadOfOrigin,