diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index 6d8acc05d3..61173613ea 100644 --- a/packages/app/e2e/browser/changes-pane.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -262,24 +262,74 @@ test.afterEach(async () => { } }); -test("Changes opens the populated committed comparison for a clean checkout", async ({ page }) => { +test("Changes opens All changes for a clean checkout", async ({ page }) => { const workspace = await createWorkspaceWithCommittedDiff(); await openWorkspaceChangesSurface(page, workspace, 90_000); const panel = page.getByTestId("working-diff-panel").filter({ visible: true }); const tree = page.getByTestId("changes-tree-panel").filter({ visible: true }); - await expect(tree.getByTestId("changes-diff-status-trigger")).toContainText("Committed"); + await expect(tree.getByTestId("changes-diff-status-trigger")).toContainText("All changes"); await expect(panel.getByTestId("diff-file-0")).toHaveAccessibleName("committed-only.ts, +1, -0"); }); -test("Changes expires a manual comparison when checkout dirtiness changes", async ({ page }) => { +test("All changes combines committed and pending edits and preserves an unfinished review", async ({ + page, + context, +}) => { + const workspace = await createWorkspaceWithNetDiff(); + await context.grantPermissions(["clipboard-read", "clipboard-write"]); + await configureDiffPresentation(page, { layout: "unified", wrapLines: false }); + await openWorkspaceChangesSurface(page, workspace); + const tree = page.getByTestId("changes-tree-panel").filter({ visible: true }); + const panel = page.getByTestId("working-diff-panel").filter({ visible: true }); + const mode = tree.getByRole("button", { name: "Diff mode", exact: true }); + await expect(mode).toContainText("All changes"); + await expectNetComparisonFiles(page, ["a-tracked.ts", "b-committed.ts"]); + await expectRenderedAddedLine(page, "committed"); + + await test.step("an unfinished review survives the first dirty edit", async () => { + await startReviewOnFirstChangedLine(page); + await page + .getByRole("textbox", { name: "Review comment" }) + .fill("Unfinished review stays here"); + await writeFile(path.join(workspace.repoPath, "c-untracked.txt"), "untracked\n"); + await expectNetComparisonFiles(page, ["a-tracked.ts", "b-committed.ts", "c-untracked.txt"]); + await expect(mode).toContainText("All changes"); + await expect(page.getByRole("textbox", { name: "Review comment" })).toHaveValue( + "Unfinished review stays here", + ); + await cancelInlineReview(page); + }); + await test.step("pending edits form a live net patch over the committed edit", async () => { + await writeFile(path.join(workspace.repoPath, "a-tracked.ts"), "working\n"); + await expectRenderedAddedLine(page, "working"); + await expect(panel.getByTestId("diff-file-0")).toHaveAccessibleName("a-tracked.ts, +1, -1"); + }); + await test.step("committing the same contents preserves the rendered comparison", async () => { + execFileSync("git", ["add", "--all"], { cwd: workspace.repoPath }); + execFileSync("git", ["commit", "-m", "Commit pending changes"], { cwd: workspace.repoPath }); + await expect(page.getByTestId("changes-primary-cta")).toContainText("Merge", { + timeout: 30_000, + }); + await expect(mode).toContainText("All changes"); + await expectNetComparisonFiles(page, ["a-tracked.ts", "b-committed.ts", "c-untracked.txt"]); + await expectRenderedAddedLine(page, "working"); + }); + await test.step("undoing to the comparison base removes the net change", async () => { + await writeFile(path.join(workspace.repoPath, "a-tracked.ts"), "base\n"); + await expectNetComparisonFiles(page, ["b-committed.ts", "c-untracked.txt"]); + await expect(mode).toContainText("All changes"); + }); +}); + +test("Changes keeps a manual comparison when checkout dirtiness changes", async ({ page }) => { const workspace = await createWorkspaceWithMountedTabDiff(); await openWorkspaceChanges(page, workspace); const tree = page.getByTestId("changes-tree-panel").filter({ visible: true }); const mode = tree.getByTestId("changes-diff-status-trigger"); - await expect(mode).toContainText("Uncommitted"); + await expect(mode).toContainText("All changes"); await mode.click(); await page.getByTestId("changes-diff-mode-committed").click(); @@ -294,7 +344,23 @@ test("Changes expires a manual comparison when checkout dirtiness changes", asyn }); await writeFile(path.join(workspace.repoPath, "new-working-change.txt"), "uncommitted\n"); - await expect(mode).toContainText("Uncommitted", { timeout: 30_000 }); + await expect(tree.getByRole("button", { name: "See uncommitted changes" })).toBeVisible({ + timeout: 30_000, + }); + await expect(mode).toContainText("Committed"); + + await selectChangesComparison(page, "Uncommitted"); + execFileSync("git", ["add", "--all"], { cwd: workspace.repoPath }); + execFileSync("git", ["commit", "-m", "Commit new working change"], { cwd: workspace.repoPath }); + await expect(tree.getByText("No changes to display", { exact: true })).toBeVisible({ + timeout: 30_000, + }); + await expect(mode).toContainText("Uncommitted"); + await writeFile(path.join(workspace.repoPath, "another-working-change.txt"), "pending\n"); + await expect(tree.getByText("another-working-change.txt", { exact: true })).toBeVisible({ + timeout: 30_000, + }); + await expect(mode).toContainText("Uncommitted"); }); test("an empty Changes comparison links to the populated comparison", async ({ page }) => { @@ -633,6 +699,7 @@ test("changes context menus expose folder revert and restore a file after confir const workspace = await createWorkspaceWithMountedTabDiff(); await useUnwrappedDiffLines(page); await openWorkspaceChanges(page, workspace); + await selectChangesComparison(page, "Uncommitted"); const tree = changesTree(page); await tree.getByTestId("diff-folder-src-toggle").click({ button: "right" }); @@ -680,6 +747,7 @@ test("discarding a staged rename restores its source path", async ({ page }) => const workspace = await createWorkspaceWithMountedTabDiff({ includeRenamedFile: true }); await useUnwrappedDiffLines(page); await openWorkspaceChanges(page, workspace); + await selectChangesComparison(page, "Uncommitted"); const panel = page.getByTestId("working-diff-panel").filter({ visible: true }); const renamedHeader = diffHeaderForPath(panel, "src/zz-renamed.ts"); @@ -710,6 +778,7 @@ test("discarding an untracked file removes it from the working tree", async ({ p const workspace = await createWorkspaceWithMountedTabDiff({ includeUntrackedFile: true }); await useUnwrappedDiffLines(page); await openWorkspaceChanges(page, workspace); + await selectChangesComparison(page, "Uncommitted"); const panel = page.getByTestId("working-diff-panel").filter({ visible: true }); const untrackedHeader = diffHeaderForPath(panel, "zz-untracked.txt"); @@ -741,6 +810,7 @@ test("shows a revert error returned by the daemon", async ({ page }) => { await failNextDiscardRequest(page); await useUnwrappedDiffLines(page); await openWorkspaceChanges(page, workspace); + await selectChangesComparison(page, "Uncommitted"); await page.getByTestId("diff-file-0-toggle").click({ button: "right" }); const confirmation = new Promise((resolve) => { @@ -892,6 +962,8 @@ test("compact Explorer renders inline hunks with a saved tree-only desktop prefe { key: CHANGES_PREFERENCES_KEY }, ); const explorer = await openCompactChanges(page, workspace); + await explorer.getByRole("button", { name: "Diff mode", exact: true }).click(); + await page.getByRole("menuitem", { name: "Uncommitted", exact: true }).click(); await expect(explorer.getByTestId("git-diff-canvas")).toBeVisible(); await expect(explorer.getByTestId("diff-file-0-toggle")).toBeVisible(); await explorer.getByTestId("diff-file-0-toggle").click({ button: "right" }); @@ -1525,6 +1597,64 @@ async function createWorkspaceWithMountedTabDiff( return { id: createdWorkspace.workspace.id, repoPath: repo.path }; } +async function createWorkspaceWithNetDiff(): Promise { + const repo = await createTempGitRepo("changes-net-", { + files: [{ path: "a-tracked.ts", content: "base\n" }], + }); + const client = await connectSeedClient(); + cleanupTasks.push({ + run: async () => { + await client.close(); + await repo.cleanup(); + }, + }); + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repo.path }); + await writeFile(path.join(repo.path, "a-tracked.ts"), "committed\n"); + await writeFile(path.join(repo.path, "b-committed.ts"), "committed only\n"); + execFileSync("git", ["add", "--all"], { cwd: repo.path }); + execFileSync("git", ["commit", "-m", "Committed changes"], { cwd: repo.path }); + const created = await client.createWorkspace({ source: { kind: "directory", path: repo.path } }); + if (!created.workspace) throw new Error(created.error ?? "Failed to create net diff workspace"); + return { id: created.workspace.id, repoPath: repo.path }; +} + +async function expectNetComparisonFiles(page: Page, files: string[]): Promise { + const tree = page.getByTestId("changes-tree-panel").filter({ visible: true }); + const panel = page.getByTestId("working-diff-panel").filter({ visible: true }); + await expect(tree.locator('[data-testid^="diff-tree-file-"][data-testid$="-name"]')).toHaveText( + files, + ); + await expect(panel.getByTestId(/^diff-file-\d+$/)).toHaveCount(files.length); + for (const file of files) { + await expect(diffHeaderForPath(panel, file)).toBeVisible(); + } +} + +async function expectRenderedAddedLine(page: Page, contents: string): Promise { + await changesTree(page).getByText("a-tracked.ts", { exact: true }).click(); + await expect(async () => { + await page.keyboard.press("Escape"); + await page.evaluate(() => navigator.clipboard.writeText("")); + const body = page.getByTestId("diff-file-0-body"); + const canvas = page.getByTestId("git-diff-canvas"); + const bounds = await body.boundingBox(); + if (!bounds) throw new Error("Expanded diff body has no bounds"); + const fontSize = await canvas.evaluate((element) => + Number.parseFloat(getComputedStyle(element).fontSize), + ); + // Replacement: hunk header, removed line, added line. + await page.mouse.click(bounds.x + 120, bounds.y + Math.round(fontSize * 1.5) * 2.5, { + button: "right", + }); + const copyLine = page.getByRole("menuitem", { name: "Copy line", exact: true }); + await expect(copyLine).toBeEnabled({ timeout: 1000 }); + await copyLine.click(); + await expect + .poll(() => page.evaluate(() => navigator.clipboard.readText()), { timeout: 1000 }) + .toBe(contents); + }).toPass({ timeout: 15_000 }); +} + async function createWorkspaceWithCommittedDiff(): Promise { const repo = await createTempGitRepo("changes-committed-", { files: [{ path: "tracked.ts", content: "export const tracked = 1;\n" }], diff --git a/packages/app/src/data/push-router.test.ts b/packages/app/src/data/push-router.test.ts index 3bf9e99f9b..6599c28467 100644 --- a/packages/app/src/data/push-router.test.ts +++ b/packages/app/src/data/push-router.test.ts @@ -51,7 +51,7 @@ function createFakeClient(config: { rejectCheckoutDiffSubscribe?: boolean } = {} emit: (message: Extract) => void; subscribeCheckoutDiffCalls: Array<{ cwd: string; - compare: { mode: "uncommitted" | "base"; baseRef?: string; ignoreWhitespace?: boolean }; + compare: { mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean }; subscriptionId: string; }>; unsubscribeCheckoutDiffCalls: string[]; @@ -67,7 +67,7 @@ function createFakeClient(config: { rejectCheckoutDiffSubscribe?: boolean } = {} }; const subscribeCheckoutDiffCalls: Array<{ cwd: string; - compare: { mode: "uncommitted" | "base"; baseRef?: string; ignoreWhitespace?: boolean }; + compare: { mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean }; subscriptionId: string; }> = []; const unsubscribeCheckoutDiffCalls: string[] = []; @@ -379,12 +379,12 @@ describe("server data push router", () => { }); }); - it("subscribes active checkout diff queries and writes matching diff events", () => { + it.each(["base", "all"] as const)("routes %s diff snapshots and updates", (mode) => { const queryClient = new QueryClient(); const fake = createFakeClient(); const serverId = "server-1"; const cwd = "/repo"; - const queryKey = checkoutDiffQueryKey(serverId, cwd, "base", "main", true); + const queryKey = checkoutDiffQueryKey(serverId, cwd, mode, "main", true); const subscriptionId = `checkoutDiff:${JSON.stringify(queryKey)}`; const observer = new QueryObserver(queryClient, { queryKey, @@ -397,17 +397,18 @@ describe("server data push router", () => { serverId, subscriptionId, cwd, - compare: { mode: "base", baseRef: "main", ignoreWhitespace: true }, + compare: { mode, baseRef: "main", ignoreWhitespace: true }, }), }); const unsubscribeObserver = observer.subscribe(() => undefined); const unmount = mountServerDataPushRouter({ client: fake.client, queryClient, serverId }); + expect(fake.subscribeCheckoutDiffCalls).toHaveLength(1); const serverSubscriptionId = fake.subscribeCheckoutDiffCalls[0]!.subscriptionId; expect(fake.subscribeCheckoutDiffCalls).toEqual([ { cwd, - compare: { mode: "base", baseRef: "main", ignoreWhitespace: true }, + compare: { mode, baseRef: "main", ignoreWhitespace: true }, subscriptionId: serverSubscriptionId, }, ]); diff --git a/packages/app/src/data/push-router.ts b/packages/app/src/data/push-router.ts index 5ab3069bd0..75ac313169 100644 --- a/packages/app/src/data/push-router.ts +++ b/packages/app/src/data/push-router.ts @@ -33,7 +33,7 @@ type CheckoutDiffCachePayload = Omit { expect(client.getCheckoutStatus).toHaveBeenCalledExactlyOnceWith(cwd); }); - it("expires a manual working-diff comparison when the fetched dirty state flipped", async () => { - selectBaseComparison(true); + it("keeps a manual working-diff comparison when the fetched dirty state flips", async () => { + selectBaseComparison(); const client = { getCheckoutStatus: vi.fn(async () => checkoutStatus({ isDirty: false })) }; await fetchCheckoutStatus({ client, serverId, cwd }); - expect(resolveWorkingDiffComparison({ serverId, cwd, isDirty: false })).toBe("base"); - expect(resolveWorkingDiffComparison({ serverId, cwd, isDirty: true })).toBe("uncommitted"); + expect(resolveWorkingDiffComparison({ serverId, cwd })).toBe("base"); }); }); @@ -286,31 +284,25 @@ describe("applyCheckoutStatusUpdateFromEvent", () => { ).toBe("unauthenticated"); }); - it("expires a manual working-diff comparison when the pushed dirty state flipped", () => { - const queryClient = createQueryClient(); - selectBaseComparison(false); - - applyCheckoutStatusUpdateFromEvent({ - queryClient, - serverId, - message: checkoutStatusUpdate(checkoutStatus({ isDirty: true })), - }); - - expect(resolveWorkingDiffComparison({ serverId, cwd, isDirty: true })).toBe("uncommitted"); - }); - - it("keeps a manual working-diff comparison while the pushed dirty state still matches", () => { - const queryClient = createQueryClient(); - selectBaseComparison(true); - - applyCheckoutStatusUpdateFromEvent({ - queryClient, - serverId, - message: checkoutStatusUpdate(checkoutStatus({ isDirty: true })), - }); - - expect(resolveWorkingDiffComparison({ serverId, cwd, isDirty: true })).toBe("base"); - }); + it.each(["base", "uncommitted"] as const)( + "keeps %s selected across dirty and clean pushes", + (comparison) => { + const queryClient = createQueryClient(); + selectWorkingDiffComparison({ serverId, cwd, comparison }); + applyCheckoutStatusUpdateFromEvent({ + queryClient, + serverId, + message: checkoutStatusUpdate(checkoutStatus({ isDirty: true })), + }); + expect(resolveWorkingDiffComparison({ serverId, cwd })).toBe(comparison); + applyCheckoutStatusUpdateFromEvent({ + queryClient, + serverId, + message: checkoutStatusUpdate(checkoutStatus({ isDirty: false })), + }); + expect(resolveWorkingDiffComparison({ serverId, cwd })).toBe(comparison); + }, + ); it("invalidates PR detail queries when the prStatus changes, ignoring the volatile requestId", () => { const queryClient = createQueryClient(); diff --git a/packages/app/src/git/checkout-status-cache.ts b/packages/app/src/git/checkout-status-cache.ts index c06499fef5..ca5cbb3a7a 100644 --- a/packages/app/src/git/checkout-status-cache.ts +++ b/packages/app/src/git/checkout-status-cache.ts @@ -9,7 +9,6 @@ import { } from "@/git/query-keys"; import { type CheckoutPrStatusPayload, normalizeCheckoutPrStatusPayload } from "@/git/pr-status"; import { resetDraftAgentCommandsForCheckout } from "@/hooks/agent-commands-query"; -import { expireWorkingDiffComparisons } from "@/git/working-diff-comparison"; export type CheckoutStatusPayload = CheckoutStatusResponse["payload"]; export type { CheckoutPrStatusPayload } from "@/git/pr-status"; @@ -18,13 +17,8 @@ export interface CheckoutStatusClient { getCheckoutStatus: (cwd: string) => Promise; } -// Checkout status enters the app through exactly two doors: daemon pushes -// (applyCheckoutStatusUpdateFromEvent) and query fetches (fetchCheckoutStatus). Both run -// the dirty-state reactions, so they hold regardless of which screens are mounted. - export async function fetchCheckoutStatus({ client, - serverId, cwd, }: { client: CheckoutStatusClient; @@ -32,7 +26,6 @@ export async function fetchCheckoutStatus({ cwd: string; }): Promise { const payload = await client.getCheckoutStatus(cwd); - expireWorkingDiffComparisons({ serverId, cwd, isDirty: payload.isGit && payload.isDirty }); return payload; } @@ -78,11 +71,6 @@ export function applyCheckoutStatusUpdateFromEvent({ void queryClient.invalidateQueries({ queryKey: checkoutCommitsQueryKey(serverId, payload.cwd), }); - expireWorkingDiffComparisons({ - serverId, - cwd: payload.cwd, - isDirty: payload.isGit && payload.isDirty, - }); if (!prStatus) { return; diff --git a/packages/app/src/git/diff-pane.tsx b/packages/app/src/git/diff-pane.tsx index 22a5c2df05..3dd84f256a 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -133,7 +133,7 @@ function useDiscardChangesAction({ }: { serverId: string; cwd: string; - diffMode: "uncommitted" | "base"; + diffMode: "uncommitted" | "base" | "all"; }): ((path: string, oldPath?: string) => void) | undefined { const { t } = useTranslation(); const toast = useToast(); @@ -281,11 +281,12 @@ export function DiffLayoutToggle({ } interface DiffModeMenuProps { - diffMode: "uncommitted" | "base"; + diffMode: "uncommitted" | "base" | "all"; committedDescription?: string; testIDPrefix?: string; onSelectUncommitted: () => void; onSelectBase: () => void; + onSelectAll: () => void; } export function DiffModeMenu({ @@ -294,10 +295,12 @@ export function DiffModeMenu({ testIDPrefix = "changes-diff", onSelectUncommitted, onSelectBase, + onSelectAll, }: DiffModeMenuProps) { const { t } = useTranslation(); const uncommittedLabel = t("workspace.git.diff.uncommitted"); const committedLabel = t("workspace.git.diff.committed"); + const allLabel = t("workspace.git.diff.all"); return ( - {diffMode === "uncommitted" ? uncommittedLabel : committedLabel} + {{ all: allLabel, uncommitted: uncommittedLabel, base: committedLabel }[diffMode]} @@ -321,6 +324,14 @@ export function DiffModeMenu({ }} + + {allLabel} + + void; + onSelectAll: () => void; onSelectUncommitted: () => void; } @@ -488,11 +500,12 @@ interface BuildChangesHeaderModelInput { committedDescription?: string; compact: boolean; cwd: string; - diffMode: "uncommitted" | "base"; + diffMode: "uncommitted" | "base" | "all"; gitActions: GitActions; mode: ChangesToolbarMode; onOpenPullRequest: () => void; onSelectBase: () => void; + onSelectAll: () => void; onSelectUncommitted: () => void; pullRequest: PrHint | null; selectedDiffStat: { additions: number; deletions: number } | null; @@ -521,6 +534,7 @@ function buildChangesHeaderModel(input: BuildChangesHeaderModelInput): { mode: input.mode, selectedDiffStat: input.selectedDiffStat, onSelectBase: input.onSelectBase, + onSelectAll: input.onSelectAll, onSelectUncommitted: input.onSelectUncommitted, }, }; @@ -737,6 +751,7 @@ function ChangesComparisonToolbar({ committedDescription={model.committedDescription} onSelectUncommitted={model.onSelectUncommitted} onSelectBase={model.onSelectBase} + onSelectAll={model.onSelectAll} /> {model.selectedDiffStat ? ( [number]; function normalizeCheckoutDiffCompare(compare: { - mode: "uncommitted" | "base"; + mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean; -}): { mode: "uncommitted" | "base"; baseRef?: string; ignoreWhitespace?: boolean } { +}): { mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean } { const ignoreWhitespace = compare.ignoreWhitespace === true; if (compare.mode === "uncommitted") { return { mode: "uncommitted", ignoreWhitespace }; } const trimmedBaseRef = compare.baseRef?.trim(); return trimmedBaseRef - ? { mode: "base", baseRef: trimmedBaseRef, ignoreWhitespace } - : { mode: "base", ignoreWhitespace }; + ? { mode: compare.mode, baseRef: trimmedBaseRef, ignoreWhitespace } + : { mode: compare.mode, ignoreWhitespace }; } export function useCheckoutDiffQuery({ @@ -49,6 +50,11 @@ export function useCheckoutDiffQuery({ enabled = true, queryScope, }: UseCheckoutDiffQueryOptions) { + // COMPAT(checkoutDiffAll): added in v0.12.0, remove after 2027-04-21 once host floor includes All changes. + const allSupported = useSessionStore( + (state) => state.sessions[serverId]?.serverInfo?.features?.checkoutDiffAll === true, + ); + const requiresUpdate = mode === "all" && !allSupported; const retainedPanelActive = useRetainedPanelActive(); const queryEnabled = enabled && retainedPanelActive; const isConnected = useHostRuntimeIsConnected(serverId); @@ -71,7 +77,7 @@ export function useCheckoutDiffQuery({ return normalizedScope ? [...comparisonKey, "scope", normalizedScope] : comparisonKey; }, [serverId, cwd, compareMode, compareBaseRef, compareIgnoreWhitespace, queryScope]); const subscriptionId = useMemo(() => `checkoutDiff:${JSON.stringify(queryKey)}`, [queryKey]); - const routeEnabled = Boolean(queryEnabled && isConnected && cwd); + const routeEnabled = Boolean(queryEnabled && isConnected && cwd && !requiresUpdate); const query = useReplicaQuery({ queryKey, @@ -91,7 +97,16 @@ export function useCheckoutDiffQuery({ }), }); - return deriveCheckoutDiffResult(query.data ?? null); + return deriveCheckoutDiffResult( + requiresUpdate && isConnected + ? { + requestId: "unsupported", + cwd, + files: [], + error: { code: "UNKNOWN", message: "Update the host to view All changes" }, + } + : (query.data ?? null), + ); } export interface CheckoutDiffResult { diff --git a/packages/app/src/git/use-working-diff.ts b/packages/app/src/git/use-working-diff.ts index daeda1479d..f835d44f53 100644 --- a/packages/app/src/git/use-working-diff.ts +++ b/packages/app/src/git/use-working-diff.ts @@ -45,7 +45,6 @@ export function useWorkingDiff({ status?.error?.message ?? (isStatusError && statusError instanceof Error ? statusError.message : null); const baseRef = gitStatus?.baseRef ?? undefined; - const hasUncommittedChanges = Boolean(gitStatus?.isDirty); const currentBranchName = gitStatus?.currentBranch && gitStatus.currentBranch !== "HEAD" ? gitStatus.currentBranch : null; @@ -53,9 +52,9 @@ export function useWorkingDiff({ serverId, workspaceId, cwd, - isDirty: hasUncommittedChanges, }); const selectUncommitted = useCallback(() => selectComparison("uncommitted"), [selectComparison]); + const selectAll = useCallback(() => selectComparison("all"), [selectComparison]); const selectBase = useCallback(() => selectComparison("base"), [selectComparison]); const { @@ -111,6 +110,7 @@ export function useWorkingDiff({ diffMode, selectUncommitted, selectBase, + selectAll, files, diffPayloadError, diffTooLarge, diff --git a/packages/app/src/git/working-diff-comparison/index.ts b/packages/app/src/git/working-diff-comparison/index.ts index bee2c81b22..84a4b461d2 100644 --- a/packages/app/src/git/working-diff-comparison/index.ts +++ b/packages/app/src/git/working-diff-comparison/index.ts @@ -1,7 +1,6 @@ import { useCallback } from "react"; import { create } from "zustand"; import { - expireWorkingDiffComparisonsInState, resolveWorkingDiffComparisonFromState, selectWorkingDiffComparisonInState, type WorkingDiffCheckoutIdentity, @@ -13,7 +12,6 @@ interface WorkingDiffComparisonStore extends WorkingDiffComparisonState { select: ( input: WorkingDiffCheckoutIdentity & { comparison: WorkingDiffComparison; - isDirty: boolean; }, ) => void; } @@ -23,21 +21,18 @@ const useWorkingDiffComparisonStore = create((set) = select: (input) => set((state) => selectWorkingDiffComparisonInState(state, input)), })); -export function useWorkingDiffComparison( - input: WorkingDiffCheckoutIdentity & { isDirty: boolean }, -): { +export function useWorkingDiffComparison(input: WorkingDiffCheckoutIdentity): { comparison: WorkingDiffComparison; selectComparison: (comparison: WorkingDiffComparison) => void; } { - const { serverId, workspaceId, cwd, isDirty } = input; + const { serverId, workspaceId, cwd } = input; const comparison = useWorkingDiffComparisonStore((state) => - resolveWorkingDiffComparisonFromState(state, { serverId, workspaceId, cwd, isDirty }), + resolveWorkingDiffComparisonFromState(state, { serverId, workspaceId, cwd }), ); const select = useWorkingDiffComparisonStore((state) => state.select); const selectComparison = useCallback( - (next: WorkingDiffComparison) => - select({ serverId, workspaceId, cwd, isDirty, comparison: next }), - [cwd, isDirty, select, serverId, workspaceId], + (next: WorkingDiffComparison) => select({ serverId, workspaceId, cwd, comparison: next }), + [cwd, select, serverId, workspaceId], ); return { comparison, selectComparison }; } @@ -45,28 +40,17 @@ export function useWorkingDiffComparison( export function selectWorkingDiffComparison( input: WorkingDiffCheckoutIdentity & { comparison: WorkingDiffComparison; - isDirty: boolean; }, ): void { useWorkingDiffComparisonStore.getState().select(input); } export function resolveWorkingDiffComparison( - input: WorkingDiffCheckoutIdentity & { isDirty: boolean }, + input: WorkingDiffCheckoutIdentity, ): WorkingDiffComparison { return resolveWorkingDiffComparisonFromState(useWorkingDiffComparisonStore.getState(), input); } -export function expireWorkingDiffComparisons(input: { - serverId: string; - cwd: string; - isDirty: boolean; -}): void { - useWorkingDiffComparisonStore.setState((state) => - expireWorkingDiffComparisonsInState(state, input), - ); -} - export function resetWorkingDiffComparisons(): void { useWorkingDiffComparisonStore.setState({ overrides: {} }); } diff --git a/packages/app/src/git/working-diff-comparison/state.test.ts b/packages/app/src/git/working-diff-comparison/state.test.ts index f86038bdc1..95fbe47ffb 100644 --- a/packages/app/src/git/working-diff-comparison/state.test.ts +++ b/packages/app/src/git/working-diff-comparison/state.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from "vitest"; import { - expireWorkingDiffComparisonsInState, resolveWorkingDiffComparisonFromState, selectWorkingDiffComparisonInState, type WorkingDiffComparisonState, @@ -26,79 +25,22 @@ describe("working diff comparison", () => { ); }); - it("defaults from checkout dirtiness and honors a matching manual selection", () => { - expect( - resolveWorkingDiffComparisonFromState(emptyState(), { ...checkout, isDirty: true }), - ).toBe("uncommitted"); - expect( - resolveWorkingDiffComparisonFromState(emptyState(), { ...checkout, isDirty: false }), - ).toBe("base"); - - const selected = selectWorkingDiffComparisonInState(emptyState(), { - ...checkout, - comparison: "base", - isDirty: true, - }); - expect(resolveWorkingDiffComparisonFromState(selected, { ...checkout, isDirty: true })).toBe( - "base", - ); + it.each([true, false])("defaults to All changes when checkout dirtiness is %s", (isDirty) => { + const status = { ...checkout, isDirty }; + expect(resolveWorkingDiffComparisonFromState(emptyState(), status)).toBe("all"); }); - it("masks and expires stale selections for every workspace on the checkout", () => { - let state = selectWorkingDiffComparisonInState(emptyState(), { - ...checkout, - comparison: "base", - isDirty: true, - }); - state = selectWorkingDiffComparisonInState(state, { - ...checkout, - workspaceId: "workspace-2", - comparison: "uncommitted", - isDirty: false, - }); - state = selectWorkingDiffComparisonInState(state, { - serverId: "server-2", - workspaceId: "workspace-3", - cwd: "/other", - comparison: "base", - isDirty: true, - }); - - expect(resolveWorkingDiffComparisonFromState(state, { ...checkout, isDirty: false })).toBe( - "base", - ); - const expired = expireWorkingDiffComparisonsInState(state, { - serverId: checkout.serverId, - cwd: checkout.cwd, - isDirty: false, - }); - expect(expired.overrides[workingDiffComparisonKey(checkout)]).toBeUndefined(); - expect( - expired.overrides[workingDiffComparisonKey({ ...checkout, workspaceId: "workspace-2" })], - ).toBeDefined(); - expect( - expired.overrides[ - workingDiffComparisonKey({ - serverId: "server-2", - workspaceId: "workspace-3", - cwd: "/other", - }) - ], - ).toBeDefined(); - }); - - it("keeps state identity when nothing expires", () => { - const state = selectWorkingDiffComparisonInState(emptyState(), { - ...checkout, - comparison: "base", - isDirty: true, - }); - expect( - expireWorkingDiffComparisonsInState(state, { - serverId: checkout.serverId, - cwd: checkout.cwd, - isDirty: true, - }), - ).toBe(state); - }); + it.each(["all", "base", "uncommitted"] as const)( + "keeps the selected %s comparison", + (comparison) => { + const selected = selectWorkingDiffComparisonInState(emptyState(), { + ...checkout, + comparison, + }); + expect(resolveWorkingDiffComparisonFromState(selected, checkout)).toBe(comparison); + expect( + resolveWorkingDiffComparisonFromState(selected, { ...checkout, workspaceId: "other" }), + ).toBe("all"); + }, + ); }); diff --git a/packages/app/src/git/working-diff-comparison/state.ts b/packages/app/src/git/working-diff-comparison/state.ts index 5c7c141376..cf077a7316 100644 --- a/packages/app/src/git/working-diff-comparison/state.ts +++ b/packages/app/src/git/working-diff-comparison/state.ts @@ -1,10 +1,7 @@ -export type WorkingDiffComparison = "uncommitted" | "base"; +export type WorkingDiffComparison = "uncommitted" | "base" | "all"; export interface WorkingDiffComparisonOverride { - serverId: string; - cwd: string; comparison: WorkingDiffComparison; - isDirtyAtSelection: boolean; } export interface WorkingDiffComparisonState { @@ -34,17 +31,13 @@ export function selectWorkingDiffComparisonInState( state: WorkingDiffComparisonState, input: WorkingDiffCheckoutIdentity & { comparison: WorkingDiffComparison; - isDirty: boolean; }, ): WorkingDiffComparisonState { return { overrides: { ...state.overrides, [workingDiffComparisonKey(input)]: { - serverId: input.serverId.trim(), - cwd: normalizeCwd(input.cwd), comparison: input.comparison, - isDirtyAtSelection: input.isDirty, }, }, }; @@ -52,34 +45,7 @@ export function selectWorkingDiffComparisonInState( export function resolveWorkingDiffComparisonFromState( state: WorkingDiffComparisonState, - input: WorkingDiffCheckoutIdentity & { isDirty: boolean }, + input: WorkingDiffCheckoutIdentity, ): WorkingDiffComparison { - const override = state.overrides[workingDiffComparisonKey(input)]; - // Status can render before boundary expiry runs, so resolution must also mask a stale - // selection under any ordering of the two updates. - if (override?.isDirtyAtSelection === input.isDirty) { - return override.comparison; - } - return input.isDirty ? "uncommitted" : "base"; -} - -export function expireWorkingDiffComparisonsInState( - state: WorkingDiffComparisonState, - input: { serverId: string; cwd: string; isDirty: boolean }, -): WorkingDiffComparisonState { - // Expire at the status boundary so selections cannot return after an unmounted surface - // misses a dirty-state transition. - const staleKeys = Object.entries(state.overrides) - .filter( - ([, override]) => - override.serverId === input.serverId.trim() && - override.cwd === normalizeCwd(input.cwd) && - override.isDirtyAtSelection !== input.isDirty, - ) - .map(([key]) => key); - if (staleKeys.length === 0) return state; - - const overrides = { ...state.overrides }; - for (const key of staleKeys) delete overrides[key]; - return { overrides }; + return state.overrides[workingDiffComparisonKey(input)]?.comparison ?? "all"; } diff --git a/packages/app/src/i18n/resources/ar.ts b/packages/app/src/i18n/resources/ar.ts index accd9cec6d..388f9be16f 100644 --- a/packages/app/src/i18n/resources/ar.ts +++ b/packages/app/src/i18n/resources/ar.ts @@ -956,6 +956,7 @@ export const ar: TranslationResources = { checkingRepository: "فحص المستودع...", notRepository: "ليس مستودع جيت", diffMode: "وضع الفرق", + all: "كل التغييرات", uncommitted: "غير ملتزم", committed: "ملتزم", branchUnknown: "مجهول", diff --git a/packages/app/src/i18n/resources/en.ts b/packages/app/src/i18n/resources/en.ts index 8002c0ff46..d64c19871d 100644 --- a/packages/app/src/i18n/resources/en.ts +++ b/packages/app/src/i18n/resources/en.ts @@ -964,6 +964,7 @@ export const en = { checkingRepository: "Checking repository...", notRepository: "Not a git repository", diffMode: "Diff mode", + all: "All changes", uncommitted: "Uncommitted", committed: "Committed", branchUnknown: "Unknown", diff --git a/packages/app/src/i18n/resources/es.ts b/packages/app/src/i18n/resources/es.ts index 464b1ab208..27c4ab8404 100644 --- a/packages/app/src/i18n/resources/es.ts +++ b/packages/app/src/i18n/resources/es.ts @@ -989,6 +989,7 @@ export const es: TranslationResources = { checkingRepository: "Comprobando repositorio...", notRepository: "No es un repositorio de git", diffMode: "modo diferencial", + all: "Todos los cambios", uncommitted: "No comprometido", committed: "Comprometido", branchUnknown: "Desconocido", diff --git a/packages/app/src/i18n/resources/fr.ts b/packages/app/src/i18n/resources/fr.ts index 4f8b43889a..29464ca9e2 100644 --- a/packages/app/src/i18n/resources/fr.ts +++ b/packages/app/src/i18n/resources/fr.ts @@ -982,6 +982,7 @@ export const fr: TranslationResources = { checkingRepository: "Vérification du dépôt…", notRepository: "Ce n’est pas un dépôt git", diffMode: "Mode de diff", + all: "Toutes les modifications", uncommitted: "Non commité", committed: "Commité", branchUnknown: "Inconnue", diff --git a/packages/app/src/i18n/resources/ja.ts b/packages/app/src/i18n/resources/ja.ts index 9fb11cb2e1..e3dba618ee 100644 --- a/packages/app/src/i18n/resources/ja.ts +++ b/packages/app/src/i18n/resources/ja.ts @@ -969,6 +969,7 @@ export const ja: TranslationResources = { checkingRepository: "リポジトリを確認中...", notRepository: "gitリポジトリではありません", diffMode: "差分モード", + all: "すべての変更", uncommitted: "未コミット", committed: "コミット済み", branchUnknown: "不明", diff --git a/packages/app/src/i18n/resources/ko.ts b/packages/app/src/i18n/resources/ko.ts index e079ae90f1..0c82504aaa 100644 --- a/packages/app/src/i18n/resources/ko.ts +++ b/packages/app/src/i18n/resources/ko.ts @@ -963,6 +963,7 @@ export const ko: TranslationResources = { checkingRepository: "저장소 확인 중...", notRepository: "Git 저장소가 아닙니다", diffMode: "Diff 모드", + all: "모든 변경 사항", uncommitted: "커밋되지 않음", committed: "커밋됨", branchUnknown: "알 수 없음", diff --git a/packages/app/src/i18n/resources/pt-BR.ts b/packages/app/src/i18n/resources/pt-BR.ts index 84a5cb7c5f..842dea8f5e 100644 --- a/packages/app/src/i18n/resources/pt-BR.ts +++ b/packages/app/src/i18n/resources/pt-BR.ts @@ -979,6 +979,7 @@ export const ptBR: TranslationResources = { checkingRepository: "Verificando repositório...", notRepository: "Não é um repositório git", diffMode: "Modo de diff", + all: "Todas as alterações", uncommitted: "Sem commit", committed: "Com commit", branchUnknown: "Desconhecido", diff --git a/packages/app/src/i18n/resources/ru.ts b/packages/app/src/i18n/resources/ru.ts index 54f0c2d6ca..7a5881c1b7 100644 --- a/packages/app/src/i18n/resources/ru.ts +++ b/packages/app/src/i18n/resources/ru.ts @@ -973,6 +973,7 @@ export const ru: TranslationResources = { checkingRepository: "Проверяем репозиторий...", notRepository: "Это не репозиторий Git", diffMode: "Режим диффа", + all: "Все изменения", uncommitted: "Незафиксированные изменения", committed: "Зафиксированные изменения", branchUnknown: "Неизвестная ветка", diff --git a/packages/app/src/i18n/resources/zh-CN.ts b/packages/app/src/i18n/resources/zh-CN.ts index 8476e55464..6cd2be6e81 100644 --- a/packages/app/src/i18n/resources/zh-CN.ts +++ b/packages/app/src/i18n/resources/zh-CN.ts @@ -948,6 +948,7 @@ export const zhCN: TranslationResources = { checkingRepository: "正在检查 repository...", notRepository: "不是 git repository", diffMode: "Diff 模式", + all: "全部更改", uncommitted: "未 commit", committed: "已 commit", branchUnknown: "未知", diff --git a/packages/app/src/review/state.ts b/packages/app/src/review/state.ts index 20fe98e098..0a1a965db5 100644 --- a/packages/app/src/review/state.ts +++ b/packages/app/src/review/state.ts @@ -1,6 +1,6 @@ import { z } from "zod"; -export type ReviewDraftMode = "uncommitted" | "base"; +export type ReviewDraftMode = "uncommitted" | "base" | "all"; export type ReviewDraftSide = "old" | "new"; export interface ReviewDraftComment { diff --git a/packages/client/src/daemon-client.ts b/packages/client/src/daemon-client.ts index 94d20585e2..de127c5b6d 100644 --- a/packages/client/src/daemon-client.ts +++ b/packages/client/src/daemon-client.ts @@ -4395,10 +4395,10 @@ export class DaemonClient { } private normalizeCheckoutDiffCompare(compare: { - mode: "uncommitted" | "base"; + mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean; - }): { mode: "uncommitted" | "base"; baseRef?: string; ignoreWhitespace?: boolean } { + }): { mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean } { if (compare.mode === "uncommitted") { return compare.ignoreWhitespace === true ? { mode: "uncommitted", ignoreWhitespace: true } @@ -4407,17 +4407,17 @@ export class DaemonClient { const trimmedBaseRef = compare.baseRef?.trim(); if (!trimmedBaseRef) { return compare.ignoreWhitespace === true - ? { mode: "base", ignoreWhitespace: true } - : { mode: "base" }; + ? { mode: compare.mode, ignoreWhitespace: true } + : { mode: compare.mode }; } return compare.ignoreWhitespace === true - ? { mode: "base", baseRef: trimmedBaseRef, ignoreWhitespace: true } - : { mode: "base", baseRef: trimmedBaseRef }; + ? { mode: compare.mode, baseRef: trimmedBaseRef, ignoreWhitespace: true } + : { mode: compare.mode, baseRef: trimmedBaseRef }; } async getCheckoutDiff( cwd: string, - compare: { mode: "uncommitted" | "base"; baseRef?: string; ignoreWhitespace?: boolean }, + compare: { mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean }, requestId?: string, ): Promise { return this.sendCorrelatedSessionRequest({ @@ -4433,7 +4433,7 @@ export class DaemonClient { observeCheckoutDiff( cwd: string, - compare: { mode: "uncommitted" | "base"; baseRef?: string; ignoreWhitespace?: boolean }, + compare: { mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean }, options?: { requestId?: string; signal?: AbortSignal }, ): OwnedSubscription { return this.observe( diff --git a/packages/protocol/src/messages.test.ts b/packages/protocol/src/messages.test.ts index ca3ab86135..373cef409b 100644 --- a/packages/protocol/src/messages.test.ts +++ b/packages/protocol/src/messages.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "vitest"; import { AgentTimelineItemPayloadSchema, + SendAgentMessageRequestSchema, FileExplorerRequestSchema, PaseoWorktreeArchiveRequestSchema, parseServerInfoStatusPayload, @@ -548,3 +549,25 @@ test("agent messages are an optional semantic payload on existing tool rows", () }).success, ).toBe(false); }); + +test.each(["all", "base", "uncommitted"] as const)( + "preserves %s review attachments while ignoring unknown attachments", + (mode) => { + const attachment = { + type: "review", + mimeType: "application/paseo-review", + cwd: "/repo", + mode, + baseRef: "main", + comments: [], + }; + const parsed = SendAgentMessageRequestSchema.parse({ + type: "send_agent_message_request", + requestId: "review", + agentId: "agent", + text: "Review", + attachments: [{ type: "future_attachment" }, attachment], + }); + expect(parsed.attachments).toEqual([attachment]); + }, +); diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index 8eff2fbbd2..3e5deeb88e 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -1168,7 +1168,7 @@ export const ReviewAttachmentSchema = z.object({ type: z.literal("review"), mimeType: z.literal("application/paseo-review"), cwd: z.string(), - mode: z.enum(["uncommitted", "base"]), + mode: z.enum(["uncommitted", "base", "all"]), baseRef: z.string().nullable().optional(), comments: z.array(ReviewAttachmentCommentSchema), }); @@ -2201,7 +2201,7 @@ const CheckoutErrorSchema = z.object({ }); const CheckoutDiffCompareSchema = z.object({ - mode: z.enum(["uncommitted", "base"]), + mode: z.enum(["uncommitted", "base", "all"]), baseRef: z.string().optional(), ignoreWhitespace: z.boolean().optional(), }); @@ -3793,6 +3793,7 @@ export const ServerInfoStatusPayloadSchema = z fsEntryDuplicate: z.boolean().optional(), // COMPAT(checkoutDiscardChanges): added in v0.3.0, remove gate after 2027-02-08. checkoutDiscardChanges: z.boolean().optional(), + checkoutDiffAll: z.boolean().optional(), // COMPAT(agentProfiles): added in v0.3.2, remove gate after 2027-02-11. // An older daemon parses its persisted config strictly, so writing // agentProfiles to one is silently dropped. The client hides the feature diff --git a/packages/server/src/server/agent/prompt-attachments.test.ts b/packages/server/src/server/agent/prompt-attachments.test.ts index 41d4a424d8..2fd022547e 100644 --- a/packages/server/src/server/agent/prompt-attachments.test.ts +++ b/packages/server/src/server/agent/prompt-attachments.test.ts @@ -69,13 +69,13 @@ describe("prompt attachments", () => { ).toContain("GitLab MR !123: Fix race in worktree setup"); }); - it("renders review attachments with compact file, line, comment, and context details", () => { + it.each(["base", "uncommitted", "all"] as const)("renders %s reviews", (mode) => { expect( renderPromptAttachmentAsText({ type: "review", mimeType: "application/paseo-review", cwd: "/tmp/repo", - mode: "base", + mode, baseRef: "main", comments: [ { @@ -111,7 +111,7 @@ describe("prompt attachments", () => { }), ).toBe( [ - "Paseo review attachment (base)", + `Paseo review attachment (${mode})`, "CWD: /tmp/repo", "Base: main", "", diff --git a/packages/server/src/server/checkout-diff-cache.test.ts b/packages/server/src/server/checkout-diff-cache.test.ts index e2ac4f31ba..37f9283320 100644 --- a/packages/server/src/server/checkout-diff-cache.test.ts +++ b/packages/server/src/server/checkout-diff-cache.test.ts @@ -114,3 +114,21 @@ test("active reads survive completed-payload eviction and failed reads can retry }), ).toEqual({ diff: "retry" }); }); + +test.each(["base", "uncommitted"] as const)( + "%s invalidation refreshes All changes without merging base-ref cache entries", + async (mode) => { + const cache = new CheckoutDiffCache(() => 0); + const read = (baseRef: string, value: string) => + cache.read({ + cwd: "repo", + compare: { mode: "all", baseRef }, + load: async () => ({ diff: value }), + }); + expect(await read("main", "main-before")).toEqual({ diff: "main-before" }); + expect(await read("next", "next-before")).toEqual({ diff: "next-before" }); + cache.invalidate("repo", mode); + expect(await read("main", "main-after")).toEqual({ diff: "main-after" }); + expect(await read("next", "next-after")).toEqual({ diff: "next-after" }); + }, +); diff --git a/packages/server/src/server/checkout-diff-cache.ts b/packages/server/src/server/checkout-diff-cache.ts index 38675bea37..4547cdf2e5 100644 --- a/packages/server/src/server/checkout-diff-cache.ts +++ b/packages/server/src/server/checkout-diff-cache.ts @@ -35,7 +35,7 @@ export class CheckoutDiffCache { const key = JSON.stringify([ cwd, compare.mode, - compare.mode === "base" ? (compare.baseRef ?? null) : null, + compare.mode !== "uncommitted" ? (compare.baseRef ?? null) : null, compare.ignoreWhitespace === true, compare.includeStructured === true, ]); @@ -83,7 +83,7 @@ export class CheckoutDiffCache { invalidate(cwd: string, mode: CheckoutDiffCompare["mode"]): void { for (const key of new Set([...this.values.keys(), ...this.active.keys()])) { const [cachedCwd, cachedMode] = JSON.parse(key) as string[]; - if (cachedCwd !== cwd || cachedMode !== mode) continue; + if (cachedCwd !== cwd || (cachedMode !== mode && cachedMode !== "all")) continue; this.values.delete(key); const pending = this.active.get(key); if (pending) pending.invalidated = true; diff --git a/packages/server/src/server/checkout-diff-manager.ts b/packages/server/src/server/checkout-diff-manager.ts index 628bb32a0a..6dc437297d 100644 --- a/packages/server/src/server/checkout-diff-manager.ts +++ b/packages/server/src/server/checkout-diff-manager.ts @@ -158,15 +158,15 @@ export class CheckoutDiffManager { } const trimmedBaseRef = compare.baseRef?.trim(); return trimmedBaseRef - ? { mode: "base", baseRef: trimmedBaseRef, ignoreWhitespace } - : { mode: "base", ignoreWhitespace }; + ? { mode: compare.mode, baseRef: trimmedBaseRef, ignoreWhitespace } + : { mode: compare.mode, ignoreWhitespace }; } private buildTargetKey(cwd: string, compare: CheckoutDiffCompareInput): string { return JSON.stringify([ cwd, compare.mode, - compare.mode === "base" ? (compare.baseRef ?? "") : "", + compare.mode !== "uncommitted" ? (compare.baseRef ?? "") : "", compare.ignoreWhitespace === true, ]); } @@ -364,7 +364,7 @@ export class CheckoutDiffManager { } private async openTarget(target: CheckoutDiffWatchTarget): Promise { - if (target.compare.mode === "base") { + if (target.compare.mode !== "uncommitted") { const snapshot = this.workspaceGitService.peekSnapshot(target.cwd) ?? (await this.workspaceGitService.getSnapshot(target.cwd, { includeForge: false })); @@ -382,7 +382,7 @@ export class CheckoutDiffManager { return; } target.workspaceGitUnsubscribe = workspaceSubscription.unsubscribe; - return; + if (target.compare.mode === "base") return; } const { repoRoot, unsubscribe } = await this.workspaceGitService.requestWorkingTreeWatch( diff --git a/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts b/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts index 75302d4168..09da7506c9 100644 --- a/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts @@ -1,5 +1,6 @@ import { execFileSync } from "node:child_process"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { expect, test } from "vitest"; @@ -158,3 +159,67 @@ test.each( }, 60_000, ); + +test("All changes subscriptions refresh on file edits and commits", async () => { + const ctx = await createDaemonTestContext(); + const cwd = mkdtempSync(join(tmpdir(), "checkout-diff-all-")); + try { + git(cwd, "init", "-b", "main"); + git(cwd, "config", "user.name", "Test"); + git(cwd, "config", "user.email", "test@example.com"); + writeFileSync(join(cwd, "file.txt"), "base\n"); + git(cwd, "add", "."); + git(cwd, "-c", "commit.gpgsign=false", "commit", "-m", "base"); + git(cwd, "checkout", "-b", "feature"); + writeFileSync(join(cwd, "file.txt"), "committed\n"); + git(cwd, "add", "."); + git(cwd, "-c", "commit.gpgsign=false", "commit", "-m", "feature"); + const subscription = ctx.client.observeCheckoutDiff(cwd, { mode: "all", baseRef: "main" }); + try { + let latest = await subscription.ready; + expect(latest.error).toBeNull(); + subscription.subscribe({ + snapshot: (snapshot) => { + latest = snapshot; + }, + update: (message) => { + if (message.type === "checkout_diff_update") latest = message.payload; + }, + }); + writeFileSync(join(cwd, "file.txt"), "working\n"); + await expect + .poll( + () => + latest.files[0]?.hunks + .flatMap((hunk) => hunk.lines) + .filter((line) => line.type === "add") + .map((line) => line.content), + { timeout: 15_000 }, + ) + .toEqual(["working"]); + writeFileSync(join(cwd, "new.txt"), "untracked\n"); + await expect + .poll(() => latest.files.map((file) => file.path), { timeout: 15_000 }) + .toEqual(["file.txt", "new.txt"]); + const beforeCommit = latest.files; + git(cwd, "add", "."); + git(cwd, "-c", "commit.gpgsign=false", "commit", "-m", "working"); + expect( + (await ctx.client.getCheckoutDiff(cwd, { mode: "all", baseRef: "main" })).files, + ).toEqual(beforeCommit); + writeFileSync(join(cwd, "file.txt"), "base\n"); + await expect + .poll(() => latest.files.map((file) => file.path), { timeout: 15_000 }) + .toEqual(["new.txt"]); + git(cwd, "checkout", "--", "file.txt"); + await expect + .poll(() => latest.files.map((file) => file.path), { timeout: 15_000 }) + .toEqual(["file.txt", "new.txt"]); + } finally { + await subscription.release(); + } + } finally { + await ctx.cleanup(); + await rm(cwd, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + } +}, 60_000); diff --git a/packages/server/src/server/websocket-server.ts b/packages/server/src/server/websocket-server.ts index dff09358a0..214fc8c9ce 100644 --- a/packages/server/src/server/websocket-server.ts +++ b/packages/server/src/server/websocket-server.ts @@ -2096,6 +2096,7 @@ export class VoiceAssistantWebSocketServer { fsEntryDuplicate: true, // COMPAT(checkoutDiscardChanges): added in v0.3.0, remove gate after 2027-02-08. checkoutDiscardChanges: true, + checkoutDiffAll: true, // COMPAT(agentProfiles): added in v0.3.2, remove gate after 2027-02-11. agentProfiles: true, // COMPAT(agentConfigApply): added in v0.3.2, remove gate after 2027-02-11. diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index a2b89bed39..be62376130 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -827,7 +827,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { private normalizeCheckoutDiffOptions(options: CheckoutDiffCompare): CheckoutDiffCompare { return { mode: options.mode, - ...(options.mode === "base" && options.baseRef !== undefined + ...(options.mode !== "uncommitted" && options.baseRef !== undefined ? { baseRef: options.baseRef } : {}), ...(options.ignoreWhitespace === true ? { ignoreWhitespace: true } : {}), diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index fe8ebdce47..0ef41b3b6f 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -1398,6 +1398,92 @@ const x = 1; expect(diff.structured).toEqual([]); }); + it.each([ + { contents: "hello\n", expectedFiles: [], patch: "" }, + { + contents: "restored\n", + expectedFiles: [ + { path: "file.txt", isNew: false, isDeleted: false, additions: 1, deletions: 1 }, + ], + patch: "-hello\n+restored\n", + }, + ])( + "compares restored untracked base files against disk: $contents", + async ({ contents, expectedFiles, patch }) => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + execFileSync("git", ["rm", "file.txt"], { cwd: repoDir }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "delete"], { + cwd: repoDir, + }); + writeFileSync(join(repoDir, "file.txt"), contents); + const indexPath = execFileSync("git", ["rev-parse", "--git-path", "index"], { + cwd: repoDir, + encoding: "utf8", + }).trim(); + const beforeIndex = readFileSync(join(repoDir, indexPath)); + const beforeStatus = execFileSync("git", ["status", "--porcelain=v1", "-z"], { + cwd: repoDir, + }); + const result = await getCheckoutDiff(repoDir, { + mode: "all", + baseRef: "main", + includeStructured: true, + }); + expect( + result.structured?.map(({ path, isNew, isDeleted, additions, deletions }) => ({ + path, + isNew, + isDeleted, + additions, + deletions, + })), + ).toEqual(expectedFiles); + expect(result.diff).toContain(patch); + expect(result.diff.length === 0).toBe(expectedFiles.length === 0); + expect(readFileSync(join(repoDir, indexPath))).toEqual(beforeIndex); + expect(execFileSync("git", ["status", "--porcelain=v1", "-z"], { cwd: repoDir })).toEqual( + beforeStatus, + ); + expect(readFileSync(join(repoDir, "file.txt"), "utf8")).toBe(contents); + expect((await getCheckoutDiff(repoDir, { mode: "base", baseRef: "main" })).diff).toContain( + "deleted file mode", + ); + expect((await getCheckoutDiff(repoDir, { mode: "uncommitted" })).diff).toContain( + "new file mode", + ); + }, + ); + + it("compares all changes as one net patch including untracked files", async () => { + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + writeFileSync(join(repoDir, "file.txt"), "committed\n"); + execFileSync("git", ["add", "."], { cwd: repoDir }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "feature"], { + cwd: repoDir, + }); + writeFileSync(join(repoDir, "file.txt"), "working\n"); + writeFileSync(join(repoDir, "new.txt"), "untracked\n"); + const compare = { mode: "all", baseRef: "main", includeStructured: true } as const; + const before = await getCheckoutDiff(repoDir, compare); + expect(before.structured?.map((file) => file.path)).toEqual(["file.txt", "new.txt"]); + expect(before.diff).toContain("-hello\n+working\n"); + expect(before.diff).not.toContain("committed"); + expect(before.diff).toContain("+untracked"); + const committed = await getCheckoutDiff(repoDir, { ...compare, mode: "base" }); + expect(committed.diff).toContain("-hello\n+committed\n"); + const uncommitted = await getCheckoutDiff(repoDir, { ...compare, mode: "uncommitted" }); + expect(uncommitted.diff).toContain("-committed\n+working\n"); + execFileSync("git", ["add", "."], { cwd: repoDir }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "working"], { + cwd: repoDir, + }); + expect((await getCheckoutDiff(repoDir, compare)).diff).toBe(before.diff); + writeFileSync(join(repoDir, "file.txt"), "hello\n"); + expect((await getCheckoutDiff(repoDir, compare)).structured?.map((file) => file.path)).toEqual([ + "new.txt", + ]); + }); + it("falls back to the local base branch when origin is absent", async () => { execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); writeFileSync(join(repoDir, "local-feature.txt"), "feature\n"); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index ce00702908..f2001e4651 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -1,7 +1,8 @@ import { MAX_RELAY_WEBSOCKET_MESSAGE_BYTES } from "@getpaseo/protocol/websocket-message-limit"; import { resolve, dirname, basename } from "path"; import { existsSync, realpathSync } from "fs"; -import { open as openFile, readFile, stat as statFile } from "fs/promises"; +import { open as openFile, readFile, stat as statFile, mkdtemp, copyFile, rm } from "fs/promises"; +import { tmpdir } from "node:os"; import { setImmediate } from "node:timers/promises"; import { TTLCache } from "@isaacs/ttlcache"; import type { CheckoutCommit, CheckoutCommitFile } from "@getpaseo/protocol/messages"; @@ -203,6 +204,7 @@ interface CheckoutFileChange { } interface CheckoutDiffRefs { + indexEnv?: { GIT_INDEX_FILE: string }; baseRef: string; targetRef?: string; includeUntracked: boolean; @@ -518,7 +520,7 @@ async function listCheckoutFileChanges( ignoreWhitespace, extra: ["-z", "--name-status", ...getCheckoutDiffRefArgs(refs)], }), - { cwd, envOverlay: READ_ONLY_GIT_ENV }, + { cwd, envOverlay: { ...READ_ONLY_GIT_ENV, ...refs.indexEnv } }, ); const nameStatusFields = nameStatusOut.split("\0"); for (let index = 0; index < nameStatusFields.length; ) { @@ -556,7 +558,7 @@ async function listCheckoutFileChanges( ["ls-files", "-z", "--others", "--exclude-standard"], { cwd, - envOverlay: READ_ONLY_GIT_ENV, + envOverlay: { ...READ_ONLY_GIT_ENV, ...refs.indexEnv }, }, ); for (const file of untrackedOut.split("\0").filter(Boolean)) { @@ -671,7 +673,7 @@ async function getTrackedNumstatByPath( }), { cwd, - envOverlay: READ_ONLY_GIT_ENV, + envOverlay: { ...READ_ONLY_GIT_ENV, ...refs.indexEnv }, maxOutputBytes: TRACKED_DIFF_NUMSTAT_MAX_BYTES, acceptExitCodes: [0], }, @@ -736,7 +738,7 @@ async function getTrackedDiffTextForPath(input: { }), { cwd: input.cwd, - envOverlay: READ_ONLY_GIT_ENV, + envOverlay: { ...READ_ONLY_GIT_ENV, ...input.refsForDiff.indexEnv }, maxOutputBytes: PER_FILE_DIFF_MAX_BYTES, }, ); @@ -779,7 +781,11 @@ async function getTrackedDiffTexts(input: { ...input.paths.map((path) => `:(literal)${path}`), ], }), - { cwd: input.cwd, envOverlay: READ_ONLY_GIT_ENV, maxOutputBytes: 8 * PER_FILE_DIFF_MAX_BYTES }, + { + cwd: input.cwd, + envOverlay: { ...READ_ONLY_GIT_ENV, ...input.refsForDiff.indexEnv }, + maxOutputBytes: 8 * PER_FILE_DIFF_MAX_BYTES, + }, ); const files = new Map(); if (!result.truncated) { @@ -905,7 +911,7 @@ export type CheckoutDiffResult = | { diff: ""; structured: []; diffTooLarge: true }; export interface CheckoutDiffCompare { - mode: "uncommitted" | "base"; + mode: "uncommitted" | "base" | "all"; baseRef?: string; ignoreWhitespace?: boolean; includeStructured?: boolean; @@ -3479,11 +3485,58 @@ async function resolveCheckoutDiffRefs( const bestBaseRef = await resolveBestComparisonBaseRef(cwd, baseRef); return { baseRef: (await tryResolveMergeBase(cwd, bestBaseRef)) ?? bestBaseRef, - targetRef: "HEAD", - includeUntracked: false, + targetRef: compare.mode === "base" ? "HEAD" : undefined, + includeUntracked: compare.mode === "all", }; } +// Git's working-tree diff ignores files absent from the index, even when they +// exist in the comparison tree. Restore those entries only in a private index. +async function prepareAllChangesIndex(cwd: string, baseRef: string) { + const untracked = await runGitCommand(["ls-files", "-z", "--others", "--exclude-standard"], { + cwd, + envOverlay: READ_ONLY_GIT_ENV, + }); + const paths = untracked.stdout.split("\0").filter(Boolean); + if (paths.length === 0) return null; + let entries = ""; + for (let start = 0; start < paths.length; start += TRACKED_DIFF_BATCH_SIZE) { + const result = await runGitCommand( + [ + "ls-tree", + "-r", + "-z", + baseRef, + "--", + ...paths.slice(start, start + TRACKED_DIFF_BATCH_SIZE).map((path) => `:(literal)${path}`), + ], + { cwd, envOverlay: READ_ONLY_GIT_ENV }, + ); + entries += result.stdout; + } + if (!entries) return null; + const directory = await mkdtemp(resolve(tmpdir(), "paseo-all-changes-")); + const indexFile = resolve(directory, "index"); + try { + const index = await runGitCommand(["rev-parse", "--git-path", "index"], { + cwd, + envOverlay: READ_ONLY_GIT_ENV, + }); + const indexPath = resolveGitRevParsePath(cwd, index.stdout); + if (!indexPath) throw new Error("Git did not return an index path"); + await copyFile(indexPath, indexFile); + await runGitCommand(["update-index", "-z", "--index-info"], { + cwd, + input: entries, + envOverlay: { ...READ_ONLY_GIT_ENV, GIT_INDEX_FILE: indexFile }, + }); + return { indexFile, cleanup: () => rm(directory, { recursive: true, force: true }) }; + } catch (error) { + await rm(directory, { recursive: true, force: true }); + throw error; + } +} + export async function getCheckoutDiff( cwd: string, compare: CheckoutDiffCompare, @@ -3496,6 +3549,23 @@ export async function getCheckoutDiff( return { diff: "" }; } + const index = + compare.mode === "all" ? await prepareAllChangesIndex(cwd, refsForDiff.baseRef) : null; + try { + return await readCheckoutDiff(cwd, compare, { + ...refsForDiff, + indexEnv: index ? { GIT_INDEX_FILE: index.indexFile } : undefined, + }); + } finally { + await index?.cleanup(); + } +} + +async function readCheckoutDiff( + cwd: string, + compare: CheckoutDiffCompare, + refsForDiff: CheckoutDiffRefs, +): Promise { const ignoreWhitespace = compare.ignoreWhitespace === true; let effectiveRefsForDiff = refsForDiff; let changes: CheckoutFileChange[];