diff --git a/packages/app/src/hooks/checkout-diff-order.test.ts b/packages/app/src/hooks/checkout-diff-order.test.ts new file mode 100644 index 000000000..abe606cb9 --- /dev/null +++ b/packages/app/src/hooks/checkout-diff-order.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { + compareCheckoutDiffPaths, + orderCheckoutDiffFiles, +} from "./checkout-diff-order"; + +function createFile(path: string, additions = 0) { + return { + path, + isNew: false, + isDeleted: false, + additions, + deletions: 0, + hunks: [], + }; +} + +describe("checkout diff ordering", () => { + it("compares paths deterministically", () => { + expect(compareCheckoutDiffPaths("a.ts", "b.ts")).toBeLessThan(0); + expect(compareCheckoutDiffPaths("b.ts", "a.ts")).toBeGreaterThan(0); + expect(compareCheckoutDiffPaths("same.ts", "same.ts")).toBe(0); + }); + + it("sorts files by path", () => { + const ordered = orderCheckoutDiffFiles([ + createFile("zeta.ts"), + createFile("alpha.ts"), + createFile("beta.ts"), + ]); + + expect(ordered.map((file) => file.path)).toEqual([ + "alpha.ts", + "beta.ts", + "zeta.ts", + ]); + }); + + it("preserves relative order for equal paths", () => { + const ordered = orderCheckoutDiffFiles([ + createFile("same.ts", 1), + createFile("same.ts", 2), + createFile("same.ts", 3), + ]); + + expect(ordered.map((file) => file.additions)).toEqual([1, 2, 3]); + }); +}); diff --git a/packages/app/src/hooks/checkout-diff-order.ts b/packages/app/src/hooks/checkout-diff-order.ts new file mode 100644 index 000000000..10abd139e --- /dev/null +++ b/packages/app/src/hooks/checkout-diff-order.ts @@ -0,0 +1,21 @@ +import type { SubscribeCheckoutDiffResponse } from "@server/shared/messages"; + +type ParsedDiffFile = SubscribeCheckoutDiffResponse["payload"]["files"][number]; + +export function compareCheckoutDiffPaths(left: string, right: string): number { + if (left === right) { + return 0; + } + return left < right ? -1 : 1; +} + +export function orderCheckoutDiffFiles( + files: ParsedDiffFile[] +): ParsedDiffFile[] { + if (files.length < 2) { + return files; + } + const ordered = [...files]; + ordered.sort((a, b) => compareCheckoutDiffPaths(a.path, b.path)); + return ordered; +} diff --git a/packages/app/src/hooks/use-checkout-diff-query.ts b/packages/app/src/hooks/use-checkout-diff-query.ts index 33c5476b5..5f7a45652 100644 --- a/packages/app/src/hooks/use-checkout-diff-query.ts +++ b/packages/app/src/hooks/use-checkout-diff-query.ts @@ -1,9 +1,10 @@ import { useQuery, useQueryClient } from "@tanstack/react-query"; -import { useCallback, useEffect } from "react"; +import { useCallback, useEffect, useId, useMemo } from "react"; import { UnistylesRuntime } from "react-native-unistyles"; import { useSessionStore } from "@/stores/session-store"; import { usePanelStore } from "@/stores/panel-store"; -import type { CheckoutDiffResponse } from "@server/shared/messages"; +import type { SubscribeCheckoutDiffResponse } from "@server/shared/messages"; +import { orderCheckoutDiffFiles } from "./checkout-diff-order"; const CHECKOUT_DIFF_STALE_TIME = 30_000; @@ -24,11 +25,29 @@ interface UseCheckoutDiffQueryOptions { enabled?: boolean; } -export type ParsedDiffFile = CheckoutDiffResponse["payload"]["files"][number]; +type CheckoutDiffQueryPayload = Omit< + SubscribeCheckoutDiffResponse["payload"], + "subscriptionId" +>; + +export type ParsedDiffFile = CheckoutDiffQueryPayload["files"][number]; export type DiffHunk = ParsedDiffFile["hunks"][number]; export type DiffLine = DiffHunk["lines"][number]; export type HighlightToken = NonNullable[number]; +function normalizeCheckoutDiffCompare(compare: { + mode: "uncommitted" | "base"; + baseRef?: string; +}): { mode: "uncommitted" | "base"; baseRef?: string } { + if (compare.mode === "uncommitted") { + return { mode: "uncommitted" }; + } + const trimmedBaseRef = compare.baseRef?.trim(); + return trimmedBaseRef + ? { mode: "base", baseRef: trimmedBaseRef } + : { mode: "base" }; +} + export function useCheckoutDiffQuery({ serverId, cwd, @@ -49,29 +68,142 @@ export function useCheckoutDiffQuery({ const desktopFileExplorerOpen = usePanelStore((state) => state.desktop.fileExplorerOpen); const explorerTab = usePanelStore((state) => state.explorerTab); const isOpen = isMobile ? mobileView === "file-explorer" : desktopFileExplorerOpen; + const hookInstanceId = useId(); + const normalizedCompare = useMemo( + () => normalizeCheckoutDiffCompare({ mode, baseRef }), + [mode, baseRef] + ); + const compareMode = normalizedCompare.mode; + const compareBaseRef = normalizedCompare.baseRef; + const queryKey = useMemo( + () => checkoutDiffQueryKey(serverId, cwd, mode, baseRef), + [serverId, cwd, mode, baseRef] + ); const query = useQuery({ - queryKey: checkoutDiffQueryKey(serverId, cwd, mode, baseRef), + queryKey, queryFn: async () => { if (!client) { throw new Error("Daemon client not available"); } - return await client.getCheckoutDiff(cwd, { mode, baseRef }); + const payload = await client.getCheckoutDiff(cwd, { + mode: compareMode, + baseRef: compareBaseRef, + }); + return { + ...payload, + files: orderCheckoutDiffFiles(payload.files), + }; }, enabled: !!client && isConnected && !!cwd && enabled, staleTime: CHECKOUT_DIFF_STALE_TIME, - refetchInterval: 10_000, }); - // Revalidate when sidebar opens with "changes" tab active useEffect(() => { - if (!isOpen || explorerTab !== "changes" || !cwd) { + if (!client || !isConnected || !cwd || !enabled) { return; } - queryClient.invalidateQueries({ - queryKey: checkoutDiffQueryKey(serverId, cwd, mode, baseRef), + if (!isOpen || explorerTab !== "changes") { + return; + } + + const subscriptionId = [ + "checkoutDiff", + hookInstanceId, + serverId, + cwd, + compareMode, + compareBaseRef ?? "", + ].join(":"); + let cancelled = false; + + const unsubscribeUpdate = client.on("checkout_diff_update", (message) => { + if (message.type !== "checkout_diff_update") { + return; + } + if (message.payload.subscriptionId !== subscriptionId) { + return; + } + queryClient.setQueryData(queryKey, { + cwd: message.payload.cwd, + files: orderCheckoutDiffFiles(message.payload.files), + error: message.payload.error, + requestId: `subscription:${subscriptionId}`, + }); }); - }, [isOpen, explorerTab, serverId, cwd, mode, baseRef, queryClient]); + const unsubscribeSubscribeResponse = client.on( + "subscribe_checkout_diff_response", + (message) => { + if (message.type !== "subscribe_checkout_diff_response") { + return; + } + if (message.payload.subscriptionId !== subscriptionId) { + return; + } + queryClient.setQueryData(queryKey, { + cwd: message.payload.cwd, + files: orderCheckoutDiffFiles(message.payload.files), + error: message.payload.error, + requestId: message.payload.requestId, + }); + } + ); + + void client + .subscribeCheckoutDiff( + cwd, + { + mode: compareMode, + baseRef: compareBaseRef, + }, + { subscriptionId } + ) + .then((payload) => { + if (cancelled) { + return; + } + queryClient.setQueryData(queryKey, { + cwd: payload.cwd, + files: orderCheckoutDiffFiles(payload.files), + error: payload.error, + requestId: payload.requestId, + }); + }) + .catch((error) => { + if (cancelled) { + return; + } + console.error("[useCheckoutDiffQuery] subscribeCheckoutDiff failed", { + serverId, + cwd, + error, + }); + }); + + return () => { + cancelled = true; + unsubscribeUpdate(); + unsubscribeSubscribeResponse(); + try { + client.unsubscribeCheckoutDiff(subscriptionId); + } catch { + // Ignore disconnect race during effect cleanup. + } + }; + }, [ + client, + isConnected, + cwd, + enabled, + isOpen, + explorerTab, + hookInstanceId, + serverId, + compareMode, + compareBaseRef, + queryKey, + queryClient, + ]); const refresh = useCallback(() => { return query.refetch(); diff --git a/packages/server/src/client/daemon-client.test.ts b/packages/server/src/client/daemon-client.test.ts index 76ebf2487..d7af20621 100644 --- a/packages/server/src/client/daemon-client.test.ts +++ b/packages/server/src/client/daemon-client.test.ts @@ -151,6 +151,185 @@ describe("DaemonClient", () => { }); }); + test("subscribes to checkout diff updates via RPC handshake", async () => { + const logger = createMockLogger(); + const mock = createMockTransport(); + + const client = new DaemonClient({ + url: "ws://test", + logger, + reconnect: { enabled: false }, + transportFactory: () => mock.transport, + }); + clients.push(client); + + const connectPromise = client.connect(); + mock.triggerOpen(); + await connectPromise; + + const promise = client.subscribeCheckoutDiff( + "/tmp/project", + { mode: "uncommitted" }, + { subscriptionId: "checkout-sub-1" } + ); + + expect(mock.sent).toHaveLength(1); + const request = JSON.parse(mock.sent[0]) as { + type: "session"; + message: { + type: "subscribe_checkout_diff_request"; + subscriptionId: string; + cwd: string; + compare: { mode: "uncommitted" | "base"; baseRef?: string }; + requestId: string; + }; + }; + expect(request.message.type).toBe("subscribe_checkout_diff_request"); + expect(request.message.subscriptionId).toBe("checkout-sub-1"); + expect(request.message.cwd).toBe("/tmp/project"); + expect(request.message.compare).toEqual({ mode: "uncommitted" }); + + mock.triggerMessage( + JSON.stringify({ + type: "session", + message: { + type: "subscribe_checkout_diff_response", + payload: { + subscriptionId: "checkout-sub-1", + cwd: "/tmp/project", + files: [], + error: null, + requestId: request.message.requestId, + }, + }, + }) + ); + + await expect(promise).resolves.toEqual({ + subscriptionId: "checkout-sub-1", + cwd: "/tmp/project", + files: [], + error: null, + requestId: request.message.requestId, + }); + }); + + test("getCheckoutDiff uses one-shot subscription protocol", async () => { + const logger = createMockLogger(); + const mock = createMockTransport(); + + const client = new DaemonClient({ + url: "ws://test", + logger, + reconnect: { enabled: false }, + transportFactory: () => mock.transport, + }); + clients.push(client); + + const connectPromise = client.connect(); + mock.triggerOpen(); + await connectPromise; + + const promise = client.getCheckoutDiff("/tmp/project", { mode: "base", baseRef: "main" }); + + expect(mock.sent).toHaveLength(1); + const subscribeRequest = JSON.parse(mock.sent[0]) as { + type: "session"; + message: { + type: "subscribe_checkout_diff_request"; + subscriptionId: string; + cwd: string; + compare: { mode: "uncommitted" | "base"; baseRef?: string }; + requestId: string; + }; + }; + expect(subscribeRequest.message.type).toBe("subscribe_checkout_diff_request"); + expect(subscribeRequest.message.cwd).toBe("/tmp/project"); + expect(subscribeRequest.message.compare).toEqual({ mode: "base", baseRef: "main" }); + + mock.triggerMessage( + JSON.stringify({ + type: "session", + message: { + type: "subscribe_checkout_diff_response", + payload: { + subscriptionId: subscribeRequest.message.subscriptionId, + cwd: "/tmp/project", + files: [], + error: null, + requestId: subscribeRequest.message.requestId, + }, + }, + }) + ); + + await expect(promise).resolves.toEqual({ + cwd: "/tmp/project", + files: [], + error: null, + requestId: subscribeRequest.message.requestId, + }); + + expect(mock.sent).toHaveLength(2); + const unsubscribeRequest = JSON.parse(mock.sent[1]) as { + type: "session"; + message: { + type: "unsubscribe_checkout_diff_request"; + subscriptionId: string; + }; + }; + expect(unsubscribeRequest.message.type).toBe("unsubscribe_checkout_diff_request"); + expect(unsubscribeRequest.message.subscriptionId).toBe( + subscribeRequest.message.subscriptionId + ); + }); + + test("resubscribes checkout diff streams after reconnect", async () => { + const logger = createMockLogger(); + const mock = createMockTransport(); + + const client = new DaemonClient({ + url: "ws://test", + logger, + reconnect: { enabled: false }, + transportFactory: () => mock.transport, + }); + clients.push(client); + + const internal = client as unknown as { + checkoutDiffSubscriptions: Map< + string, + { cwd: string; compare: { mode: "uncommitted" | "base"; baseRef?: string } } + >; + }; + internal.checkoutDiffSubscriptions.set("checkout-sub-1", { + cwd: "/tmp/project", + compare: { mode: "base", baseRef: "main" }, + }); + + const connectPromise = client.connect(); + mock.triggerOpen(); + await connectPromise; + + expect(mock.sent).toHaveLength(1); + const request = JSON.parse(mock.sent[0]) as { + type: "session"; + message: { + type: "subscribe_checkout_diff_request"; + subscriptionId: string; + cwd: string; + compare: { mode: "uncommitted" | "base"; baseRef?: string }; + requestId: string; + }; + }; + expect(request.message.type).toBe("subscribe_checkout_diff_request"); + expect(request.message.subscriptionId).toBe("checkout-sub-1"); + expect(request.message.cwd).toBe("/tmp/project"); + expect(request.message.compare).toEqual({ mode: "base", baseRef: "main" }); + expect(typeof request.message.requestId).toBe("string"); + expect(request.message.requestId.length).toBeGreaterThan(0); + }); + test("fetches project-grouped agents via RPC", async () => { const logger = createMockLogger(); const mock = createMockTransport(); diff --git a/packages/server/src/client/daemon-client.ts b/packages/server/src/client/daemon-client.ts index 6bcf83e29..d9b44ee04 100644 --- a/packages/server/src/client/daemon-client.ts +++ b/packages/server/src/client/daemon-client.ts @@ -19,7 +19,6 @@ import type { GitSetupOptions, HighlightedDiffResponse, CheckoutStatusResponse, - CheckoutDiffResponse, CheckoutCommitResponse, CheckoutMergeResponse, CheckoutMergeFromBaseResponse, @@ -182,7 +181,11 @@ export type CreateAgentRequestOptions = { type GitDiffPayload = GitDiffResponse["payload"]; type HighlightedDiffPayload = HighlightedDiffResponse["payload"]; type CheckoutStatusPayload = CheckoutStatusResponse["payload"]; -type CheckoutDiffPayload = CheckoutDiffResponse["payload"]; +type SubscribeCheckoutDiffPayload = Extract< + SessionOutboundMessage, + { type: "subscribe_checkout_diff_response" } +>["payload"]; +type CheckoutDiffPayload = Omit; type CheckoutCommitPayload = CheckoutCommitResponse["payload"]; type CheckoutMergePayload = CheckoutMergeResponse["payload"]; type CheckoutMergeFromBasePayload = CheckoutMergeFromBaseResponse["payload"]; @@ -296,6 +299,10 @@ export class DaemonClient { string, { labels?: Record; agentId?: string } | undefined >(); + private checkoutDiffSubscriptions = new Map< + string, + { cwd: string; compare: { mode: "uncommitted" | "base"; baseRef?: string } } + >(); private logger: Logger; private pendingSendQueue: PendingSend[] = []; private relayClientId: string | null = null; @@ -401,6 +408,7 @@ export class DaemonClient { this.reconnectAttempt = 0; this.updateConnectionState({ status: "connected" }); this.resubscribeAgentUpdates(); + this.resubscribeCheckoutDiffSubscriptions(); this.flushPendingSendQueue(); this.resolveConnect(); }), @@ -951,6 +959,22 @@ export class DaemonClient { } } + private resubscribeCheckoutDiffSubscriptions(): void { + if (this.checkoutDiffSubscriptions.size === 0) { + return; + } + for (const [subscriptionId, subscription] of this.checkoutDiffSubscriptions) { + const message = SessionInboundMessageSchema.parse({ + type: "subscribe_checkout_diff_request", + subscriptionId, + cwd: subscription.cwd, + compare: subscription.compare, + requestId: this.createRequestId(), + }); + this.sendSessionMessage(message); + } + } + // ============================================================================ // Agent Lifecycle // ============================================================================ @@ -1521,32 +1545,101 @@ export class DaemonClient { return responsePromise; } + private normalizeCheckoutDiffCompare( + compare: { mode: "uncommitted" | "base"; baseRef?: string } + ): { mode: "uncommitted" | "base"; baseRef?: string } { + if (compare.mode === "uncommitted") { + return { mode: "uncommitted" }; + } + const trimmedBaseRef = compare.baseRef?.trim(); + if (!trimmedBaseRef) { + return { mode: "base" }; + } + return { mode: "base", baseRef: trimmedBaseRef }; + } + async getCheckoutDiff( cwd: string, compare: { mode: "uncommitted" | "base"; baseRef?: string }, requestId?: string ): Promise { - const resolvedRequestId = this.createRequestId(requestId); - const message = SessionInboundMessageSchema.parse({ - type: "checkout_diff_request", + const oneShotSubscriptionId = `oneshot-checkout-diff:${crypto.randomUUID()}`; + try { + const payload = await this.subscribeCheckoutDiff(cwd, compare, { + subscriptionId: oneShotSubscriptionId, + requestId, + }); + return { + cwd: payload.cwd, + files: payload.files, + error: payload.error, + requestId: payload.requestId, + }; + } finally { + try { + this.unsubscribeCheckoutDiff(oneShotSubscriptionId); + } catch { + // Ignore disconnect races during one-shot cleanup. + } + } + } + + async subscribeCheckoutDiff( + cwd: string, + compare: { mode: "uncommitted" | "base"; baseRef?: string }, + options?: { subscriptionId?: string; requestId?: string } + ): Promise { + const subscriptionId = options?.subscriptionId ?? crypto.randomUUID(); + const normalizedCompare = this.normalizeCheckoutDiffCompare(compare); + const previousSubscription = this.checkoutDiffSubscriptions.get(subscriptionId) ?? null; + this.checkoutDiffSubscriptions.set(subscriptionId, { cwd, - compare, + compare: normalizedCompare, + }); + + const resolvedRequestId = this.createRequestId(options?.requestId); + const message = SessionInboundMessageSchema.parse({ + type: "subscribe_checkout_diff_request", + subscriptionId, + cwd, + compare: normalizedCompare, requestId: resolvedRequestId, }); - return this.sendRequest({ - requestId: resolvedRequestId, - message, - timeout: 60000, - options: { skipQueue: true }, - select: (msg) => { - if (msg.type !== "checkout_diff_response") { - return null; - } - if (msg.payload.requestId !== resolvedRequestId) { - return null; - } - return msg.payload; - }, + + try { + return await this.sendRequest({ + requestId: resolvedRequestId, + message, + timeout: 60000, + options: { skipQueue: true }, + select: (msg) => { + if (msg.type !== "subscribe_checkout_diff_response") { + return null; + } + if (msg.payload.requestId !== resolvedRequestId) { + return null; + } + if (msg.payload.subscriptionId !== subscriptionId) { + return null; + } + return msg.payload; + }, + }); + } catch (error) { + if (previousSubscription) { + this.checkoutDiffSubscriptions.set(subscriptionId, previousSubscription); + } else { + this.checkoutDiffSubscriptions.delete(subscriptionId); + } + throw error; + } + } + + unsubscribeCheckoutDiff(subscriptionId: string): void { + this.checkoutDiffSubscriptions.delete(subscriptionId); + this.sendSessionMessage({ + type: "unsubscribe_checkout_diff_request", + subscriptionId, }); } diff --git a/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts b/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts new file mode 100644 index 000000000..e513385a2 --- /dev/null +++ b/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts @@ -0,0 +1,166 @@ +import { describe, test, expect, beforeEach, afterEach } from "vitest"; +import { execSync } from "child_process"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import path from "path"; +import { + createDaemonTestContext, + type DaemonTestContext, +} from "../test-utils/index.js"; +import type { SessionOutboundMessage } from "../messages.js"; + +type CheckoutDiffUpdatePayload = Extract< + SessionOutboundMessage, + { type: "checkout_diff_update" } +>["payload"]; + +function tmpCwd(): string { + return mkdtempSync(path.join(tmpdir(), "daemon-e2e-checkout-diff-")); +} + +function initGitRepo(cwd: string): void { + execSync("git init -b main", { cwd, stdio: "pipe" }); + execSync("git config user.email 'test@test.com'", { cwd, stdio: "pipe" }); + execSync("git config user.name 'Test'", { cwd, stdio: "pipe" }); +} + +function commitFile(cwd: string, fileName: string, content: string): void { + const filePath = path.join(cwd, fileName); + writeFileSync(filePath, content); + execSync(`git add "${fileName}"`, { cwd, stdio: "pipe" }); + execSync("git -c commit.gpgsign=false commit -m 'Initial commit'", { + cwd, + stdio: "pipe", + }); +} + +async function waitForCheckoutDiffUpdate( + ctx: DaemonTestContext, + subscriptionId: string, + predicate: (payload: CheckoutDiffUpdatePayload) => boolean, + timeoutMs = 15000 +): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + unsubscribe(); + reject( + new Error( + `Timed out waiting for checkout_diff_update (${subscriptionId})` + ) + ); + }, timeoutMs); + + const unsubscribe = ctx.client.on("checkout_diff_update", (message) => { + if (message.type !== "checkout_diff_update") { + return; + } + if (message.payload.subscriptionId !== subscriptionId) { + return; + } + if (!predicate(message.payload)) { + return; + } + clearTimeout(timeout); + unsubscribe(); + resolve(message.payload); + }); + }); +} + +describe("daemon E2E checkout diff subscriptions", () => { + let ctx: DaemonTestContext; + + beforeEach(async () => { + ctx = await createDaemonTestContext(); + }); + + afterEach(async () => { + await ctx.cleanup(); + }, 60000); + + test( + "pushes file-level checkout diff updates with deterministic path order", + async () => { + const cwd = tmpCwd(); + + try { + initGitRepo(cwd); + commitFile(cwd, "base.txt", "base\n"); + + const subscriptionId = "checkout-diff-e2e-subscription"; + const initial = await ctx.client.subscribeCheckoutDiff( + cwd, + { mode: "uncommitted" }, + { subscriptionId } + ); + + expect(initial.error).toBeNull(); + expect(initial.files).toEqual([]); + + writeFileSync(path.join(cwd, "zeta.txt"), "zeta\n"); + writeFileSync(path.join(cwd, "alpha.txt"), "alpha\n"); + + const update = await waitForCheckoutDiffUpdate( + ctx, + subscriptionId, + (payload) => { + const paths = payload.files.map((file) => file.path); + return paths.includes("alpha.txt") && paths.includes("zeta.txt"); + } + ); + + expect(update.error).toBeNull(); + expect(update.files.map((file) => file.path)).toEqual([ + "alpha.txt", + "zeta.txt", + ]); + + ctx.client.unsubscribeCheckoutDiff(subscriptionId); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }, + 60000 + ); + + test( + "pushes updates when subscribed from a subdirectory and files change outside it", + async () => { + const cwd = tmpCwd(); + + try { + initGitRepo(cwd); + commitFile(cwd, "base.txt", "base\n"); + + const nestedDir = path.join(cwd, "nested", "dir"); + mkdirSync(nestedDir, { recursive: true }); + + const subscriptionId = "checkout-diff-subdir-e2e-subscription"; + const initial = await ctx.client.subscribeCheckoutDiff( + nestedDir, + { mode: "uncommitted" }, + { subscriptionId } + ); + + expect(initial.error).toBeNull(); + expect(initial.files).toEqual([]); + + writeFileSync(path.join(cwd, "outside-subdir.txt"), "changed outside\n"); + + const update = await waitForCheckoutDiffUpdate( + ctx, + subscriptionId, + (payload) => payload.files.some((file) => file.path === "outside-subdir.txt") + ); + + expect(update.error).toBeNull(); + expect(update.files.some((file) => file.path === "outside-subdir.txt")).toBe(true); + + ctx.client.unsubscribeCheckoutDiff(subscriptionId); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }, + 60000 + ); +}); diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 0d023d447..4203be4bf 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -1,4 +1,5 @@ import { v4 as uuidv4 } from "uuid"; +import { watch, type FSWatcher } from "node:fs"; import { stat } from "fs/promises"; import { exec } from "child_process"; import { promisify } from "util"; @@ -19,6 +20,8 @@ import { type UnsubscribeTerminalRequest, type TerminalInput, type KillTerminalRequest, + type SubscribeCheckoutDiffRequest, + type UnsubscribeCheckoutDiffRequest, type ProjectCheckoutLitePayload, type ProjectPlacementPayload, } from "./messages.js"; @@ -131,6 +134,8 @@ const DEFAULT_AGENT_PROVIDER = AGENT_PROVIDER_IDS[0]; const RESTART_EXIT_DELAY_MS = 250; const PROJECT_PLACEMENT_CACHE_TTL_MS = 10_000; const MAX_AGENTS_PER_PROJECT = 5; +const CHECKOUT_DIFF_WATCH_DEBOUNCE_MS = 150; +const CHECKOUT_DIFF_FALLBACK_REFRESH_MS = 5_000; /** * Default model used for auto-generating commit messages and PR descriptions. @@ -212,6 +217,28 @@ function deriveProjectGroupingName(projectKey: string): string { type ProcessingPhase = "idle" | "transcribing"; +type CheckoutDiffCompareInput = SubscribeCheckoutDiffRequest["compare"]; + +type CheckoutDiffSnapshotPayload = Omit< + Extract["payload"], + "subscriptionId" +>; + +type CheckoutDiffWatchTarget = { + key: string; + cwd: string; + diffCwd: string; + compare: CheckoutDiffCompareInput; + subscriptions: Set; + watchers: FSWatcher[]; + fallbackRefreshInterval: NodeJS.Timeout | null; + debounceTimer: NodeJS.Timeout | null; + refreshPromise: Promise | null; + refreshQueued: boolean; + latestPayload: CheckoutDiffSnapshotPayload | null; + latestFingerprint: string | null; +}; + type NormalizedGitOptions = { baseBranch?: string; createNewBranch: boolean; @@ -455,6 +482,8 @@ export class Session { private readonly MOBILE_BACKGROUND_STREAM_GRACE_MS = 60_000; private readonly terminalManager: TerminalManager | null; private terminalSubscriptions: Map void> = new Map(); + private readonly checkoutDiffSubscriptions = new Map(); + private readonly checkoutDiffTargets = new Map(); private readonly voiceAgentMcpStdio: VoiceMcpStdioConfig | null; private readonly localSpeechModelsDir: string; private readonly defaultLocalSpeechModelIds: LocalSpeechModelId[]; @@ -1189,8 +1218,12 @@ export class Session { await this.handleValidateBranchRequest(msg); break; - case "checkout_diff_request": - await this.handleCheckoutDiffRequest(msg); + case "subscribe_checkout_diff_request": + await this.handleSubscribeCheckoutDiffRequest(msg); + break; + + case "unsubscribe_checkout_diff_request": + this.handleUnsubscribeCheckoutDiffRequest(msg); break; case "checkout_commit_request": @@ -3496,14 +3529,127 @@ export class Session { } } - private async handleCheckoutDiffRequest( - msg: Extract - ): Promise { - const { cwd, requestId, compare } = msg; + private normalizeCheckoutDiffCompare(compare: CheckoutDiffCompareInput): CheckoutDiffCompareInput { + if (compare.mode === "uncommitted") { + return { mode: "uncommitted" }; + } + const trimmedBaseRef = compare.baseRef?.trim(); + return trimmedBaseRef + ? { mode: "base", baseRef: trimmedBaseRef } + : { mode: "base" }; + } + private buildCheckoutDiffTargetKey(cwd: string, compare: CheckoutDiffCompareInput): string { + return JSON.stringify([ + cwd, + compare.mode, + compare.mode === "base" ? (compare.baseRef ?? "") : "", + ]); + } + + private closeCheckoutDiffWatchTarget(target: CheckoutDiffWatchTarget): void { + if (target.debounceTimer) { + clearTimeout(target.debounceTimer); + target.debounceTimer = null; + } + if (target.fallbackRefreshInterval) { + clearInterval(target.fallbackRefreshInterval); + target.fallbackRefreshInterval = null; + } + for (const watcher of target.watchers) { + watcher.close(); + } + target.watchers = []; + } + + private removeCheckoutDiffSubscription(subscriptionId: string): void { + const subscription = this.checkoutDiffSubscriptions.get(subscriptionId); + if (!subscription) { + return; + } + this.checkoutDiffSubscriptions.delete(subscriptionId); + + const target = this.checkoutDiffTargets.get(subscription.targetKey); + if (!target) { + return; + } + target.subscriptions.delete(subscriptionId); + if (target.subscriptions.size === 0) { + this.closeCheckoutDiffWatchTarget(target); + this.checkoutDiffTargets.delete(subscription.targetKey); + } + } + + private async resolveCheckoutGitDir(cwd: string): Promise { + try { + const { stdout } = await execAsync("git rev-parse --absolute-git-dir", { + cwd, + env: READ_ONLY_GIT_ENV, + }); + const gitDir = stdout.trim(); + return gitDir.length > 0 ? gitDir : null; + } catch { + return null; + } + } + + private async resolveCheckoutWatchRoot(cwd: string): Promise { + try { + const { stdout } = await execAsync( + "git rev-parse --path-format=absolute --show-toplevel", + { + cwd, + env: READ_ONLY_GIT_ENV, + } + ); + const root = stdout.trim(); + return root.length > 0 ? root : null; + } catch { + return null; + } + } + + private scheduleCheckoutDiffTargetRefresh(target: CheckoutDiffWatchTarget): void { + if (target.debounceTimer) { + clearTimeout(target.debounceTimer); + } + target.debounceTimer = setTimeout(() => { + target.debounceTimer = null; + void this.refreshCheckoutDiffTarget(target); + }, CHECKOUT_DIFF_WATCH_DEBOUNCE_MS); + } + + private emitCheckoutDiffUpdate( + target: CheckoutDiffWatchTarget, + snapshot: CheckoutDiffSnapshotPayload + ): void { + if (target.subscriptions.size === 0) { + return; + } + for (const subscriptionId of target.subscriptions) { + this.emit({ + type: "checkout_diff_update", + payload: { + subscriptionId, + ...snapshot, + }, + }); + } + } + + private checkoutDiffSnapshotFingerprint(snapshot: CheckoutDiffSnapshotPayload): string { + return JSON.stringify(snapshot); + } + + private async computeCheckoutDiffSnapshot( + cwd: string, + compare: CheckoutDiffCompareInput, + options?: { diffCwd?: string } + ): Promise { + const diffCwd = options?.diffCwd ?? cwd; try { const diffResult = await getCheckoutDiff( - cwd, + diffCwd, { mode: compare.mode, baseRef: compare.baseRef, @@ -3511,25 +3657,202 @@ export class Session { }, { paseoHome: this.paseoHome } ); - this.emit({ - type: "checkout_diff_response", - payload: { - cwd, - files: diffResult.structured ?? [], - error: null, - requestId, - }, + const files = [...(diffResult.structured ?? [])]; + files.sort((a, b) => { + if (a.path === b.path) return 0; + return a.path < b.path ? -1 : 1; }); + return { + cwd, + files, + error: null, + }; } catch (error) { - this.emit({ - type: "checkout_diff_response", - payload: { - cwd, - files: [], - error: this.toCheckoutError(error), - requestId, - }, + return { + cwd, + files: [], + error: this.toCheckoutError(error), + }; + } + } + + private async refreshCheckoutDiffTarget(target: CheckoutDiffWatchTarget): Promise { + if (target.refreshPromise) { + target.refreshQueued = true; + return; + } + + target.refreshPromise = (async () => { + do { + target.refreshQueued = false; + const snapshot = await this.computeCheckoutDiffSnapshot( + target.cwd, + target.compare, + { diffCwd: target.diffCwd } + ); + target.latestPayload = snapshot; + const fingerprint = this.checkoutDiffSnapshotFingerprint(snapshot); + if (fingerprint !== target.latestFingerprint) { + target.latestFingerprint = fingerprint; + this.emitCheckoutDiffUpdate(target, snapshot); + } + } while (target.refreshQueued); + })(); + + try { + await target.refreshPromise; + } finally { + target.refreshPromise = null; + } + } + + private async ensureCheckoutDiffWatchTarget( + cwd: string, + compare: CheckoutDiffCompareInput + ): Promise { + const targetKey = this.buildCheckoutDiffTargetKey(cwd, compare); + const existing = this.checkoutDiffTargets.get(targetKey); + if (existing) { + return existing; + } + + const watchRoot = await this.resolveCheckoutWatchRoot(cwd); + const target: CheckoutDiffWatchTarget = { + key: targetKey, + cwd, + diffCwd: watchRoot ?? cwd, + compare, + subscriptions: new Set(), + watchers: [], + fallbackRefreshInterval: null, + debounceTimer: null, + refreshPromise: null, + refreshQueued: false, + latestPayload: null, + latestFingerprint: null, + }; + + const watchPaths = new Set([cwd]); + if (watchRoot) { + watchPaths.add(watchRoot); + } + const gitDir = await this.resolveCheckoutGitDir(cwd); + if (gitDir) { + watchPaths.add(gitDir); + } + + let hasWatchRootCoverage = false; + for (const watchPath of watchPaths) { + const createWatcher = (recursive: boolean): FSWatcher => + watch( + watchPath, + { recursive }, + () => { + this.scheduleCheckoutDiffTargetRefresh(target); + } + ); + + let watcher: FSWatcher | null = null; + try { + watcher = createWatcher(true); + } catch (error) { + try { + watcher = createWatcher(false); + this.sessionLogger.warn( + { err: error, watchPath, cwd, compare }, + "Checkout diff recursive watch unavailable; using non-recursive fallback" + ); + } catch (fallbackError) { + this.sessionLogger.warn( + { err: fallbackError, watchPath, cwd, compare }, + "Failed to start checkout diff watcher" + ); + } + } + + if (!watcher) { + continue; + } + + watcher.on("error", (error) => { + this.sessionLogger.warn( + { err: error, watchPath, cwd, compare }, + "Checkout diff watcher error" + ); }); + target.watchers.push(watcher); + if (watchRoot && watchPath === watchRoot) { + hasWatchRootCoverage = true; + } + } + + const missingRepoCoverage = Boolean(watchRoot) && !hasWatchRootCoverage; + if (target.watchers.length === 0 || missingRepoCoverage) { + target.fallbackRefreshInterval = setInterval(() => { + this.scheduleCheckoutDiffTargetRefresh(target); + }, CHECKOUT_DIFF_FALLBACK_REFRESH_MS); + this.sessionLogger.warn( + { + cwd, + compare, + intervalMs: CHECKOUT_DIFF_FALLBACK_REFRESH_MS, + reason: + target.watchers.length === 0 + ? "no_watchers" + : "missing_repo_root_coverage", + }, + "Checkout diff watchers unavailable; using timed refresh fallback" + ); + } + + this.checkoutDiffTargets.set(targetKey, target); + return target; + } + + private async handleSubscribeCheckoutDiffRequest( + msg: SubscribeCheckoutDiffRequest + ): Promise { + const cwd = expandTilde(msg.cwd); + const compare = this.normalizeCheckoutDiffCompare(msg.compare); + + this.removeCheckoutDiffSubscription(msg.subscriptionId); + const target = await this.ensureCheckoutDiffWatchTarget(cwd, compare); + target.subscriptions.add(msg.subscriptionId); + this.checkoutDiffSubscriptions.set(msg.subscriptionId, { + targetKey: target.key, + }); + + const snapshot = + target.latestPayload ?? + (await this.computeCheckoutDiffSnapshot(cwd, compare, { + diffCwd: target.diffCwd, + })); + target.latestPayload = snapshot; + target.latestFingerprint = this.checkoutDiffSnapshotFingerprint(snapshot); + + this.emit({ + type: "subscribe_checkout_diff_response", + payload: { + subscriptionId: msg.subscriptionId, + ...snapshot, + requestId: msg.requestId, + }, + }); + } + + private handleUnsubscribeCheckoutDiffRequest( + msg: UnsubscribeCheckoutDiffRequest + ): void { + this.removeCheckoutDiffSubscription(msg.subscriptionId); + } + + private scheduleCheckoutDiffRefreshForCwd(cwd: string): void { + const resolvedCwd = expandTilde(cwd); + for (const target of this.checkoutDiffTargets.values()) { + if (target.cwd !== resolvedCwd && target.diffCwd !== resolvedCwd) { + continue; + } + this.scheduleCheckoutDiffTargetRefresh(target); } } @@ -3551,6 +3874,7 @@ export class Session { message, addAll: msg.addAll ?? true, }); + this.scheduleCheckoutDiffRefreshForCwd(cwd); this.emit({ type: "checkout_commit_response", @@ -3624,6 +3948,7 @@ export class Session { }, { paseoHome: this.paseoHome } ); + this.scheduleCheckoutDiffRefreshForCwd(cwd); this.emit({ type: "checkout_merge_response", @@ -3670,6 +3995,7 @@ export class Session { requireCleanTarget: msg.requireCleanTarget ?? true, }, ); + this.scheduleCheckoutDiffRefreshForCwd(cwd); this.emit({ type: "checkout_merge_from_base_response", @@ -5502,6 +5828,12 @@ export class Session { unsubscribe(); } this.terminalSubscriptions.clear(); + + for (const target of this.checkoutDiffTargets.values()) { + this.closeCheckoutDiffWatchTarget(target); + } + this.checkoutDiffTargets.clear(); + this.checkoutDiffSubscriptions.clear(); } // ============================================================================ diff --git a/packages/server/src/shared/messages.ts b/packages/server/src/shared/messages.ts index b0d87c752..25b5f2284 100644 --- a/packages/server/src/shared/messages.ts +++ b/packages/server/src/shared/messages.ts @@ -707,13 +707,19 @@ export const CheckoutStatusRequestSchema = z.object({ requestId: z.string(), }); -export const CheckoutDiffRequestSchema = z.object({ - type: z.literal("checkout_diff_request"), +export const SubscribeCheckoutDiffRequestSchema = z.object({ + type: z.literal("subscribe_checkout_diff_request"), + subscriptionId: z.string(), cwd: z.string(), compare: CheckoutDiffCompareSchema, requestId: z.string(), }); +export const UnsubscribeCheckoutDiffRequestSchema = z.object({ + type: z.literal("unsubscribe_checkout_diff_request"), + subscriptionId: z.string(), +}); + export const CheckoutCommitRequestSchema = z.object({ type: z.literal("checkout_commit_request"), cwd: z.string(), @@ -987,7 +993,8 @@ export const SessionInboundMessageSchema = z.discriminatedUnion("type", [ AgentPermissionResponseMessageSchema, GitDiffRequestSchema, CheckoutStatusRequestSchema, - CheckoutDiffRequestSchema, + SubscribeCheckoutDiffRequestSchema, + UnsubscribeCheckoutDiffRequestSchema, CheckoutCommitRequestSchema, CheckoutMergeRequestSchema, CheckoutMergeFromBaseRequestSchema, @@ -1455,16 +1462,25 @@ export const CheckoutStatusResponseSchema = z.object({ ]), }); -export const CheckoutDiffResponseSchema = z.object({ - type: z.literal("checkout_diff_response"), - payload: z.object({ - cwd: z.string(), - files: z.array(ParsedDiffFileSchema), - error: CheckoutErrorSchema.nullable(), +const CheckoutDiffSubscriptionPayloadSchema = z.object({ + subscriptionId: z.string(), + cwd: z.string(), + files: z.array(ParsedDiffFileSchema), + error: CheckoutErrorSchema.nullable(), +}); + +export const SubscribeCheckoutDiffResponseSchema = z.object({ + type: z.literal("subscribe_checkout_diff_response"), + payload: CheckoutDiffSubscriptionPayloadSchema.extend({ requestId: z.string(), }), }); +export const CheckoutDiffUpdateSchema = z.object({ + type: z.literal("checkout_diff_update"), + payload: CheckoutDiffSubscriptionPayloadSchema, +}); + export const CheckoutCommitResponseSchema = z.object({ type: z.literal("checkout_commit_response"), payload: z.object({ @@ -1800,7 +1816,8 @@ export const SessionOutboundMessageSchema = z.discriminatedUnion("type", [ AgentArchivedMessageSchema, GitDiffResponseSchema, CheckoutStatusResponseSchema, - CheckoutDiffResponseSchema, + SubscribeCheckoutDiffResponseSchema, + CheckoutDiffUpdateSchema, CheckoutCommitResponseSchema, CheckoutMergeResponseSchema, CheckoutMergeFromBaseResponseSchema, @@ -1909,8 +1926,16 @@ export type GitDiffRequest = z.infer; export type GitDiffResponse = z.infer; export type CheckoutStatusRequest = z.infer; export type CheckoutStatusResponse = z.infer; -export type CheckoutDiffRequest = z.infer; -export type CheckoutDiffResponse = z.infer; +export type SubscribeCheckoutDiffRequest = z.infer< + typeof SubscribeCheckoutDiffRequestSchema +>; +export type UnsubscribeCheckoutDiffRequest = z.infer< + typeof UnsubscribeCheckoutDiffRequestSchema +>; +export type SubscribeCheckoutDiffResponse = z.infer< + typeof SubscribeCheckoutDiffResponseSchema +>; +export type CheckoutDiffUpdate = z.infer; export type CheckoutCommitRequest = z.infer; export type CheckoutCommitResponse = z.infer; export type CheckoutMergeRequest = z.infer; diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 81fd6366c..49536eef1 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -942,7 +942,10 @@ export async function getCheckoutDiff( } const changes = await listCheckoutFileChanges(cwd, refForDiff); - changes.sort((a, b) => a.path.localeCompare(b.path)); + changes.sort((a, b) => { + if (a.path === b.path) return 0; + return a.path < b.path ? -1 : 1; + }); const structured: ParsedDiffFile[] = []; let diffText = "";