diff --git a/packages/server/src/server/checkout-diff-manager.test.ts b/packages/server/src/server/checkout-diff-manager.test.ts index 565887cd93..1335d4abec 100644 --- a/packages/server/src/server/checkout-diff-manager.test.ts +++ b/packages/server/src/server/checkout-diff-manager.test.ts @@ -193,3 +193,61 @@ test("legacy diffs retain successful bodies when an oversized sibling is removed subscription.unsubscribe(); }); }); + +test("legacy subscriptions reselect the preferred base when a remote ref appears", async () => { + await withCheckout(async ({ cwd, manager, git }) => { + git("checkout", "-b", "feature"); + await writeFile(join(cwd, "b.ts"), "export const b = 1;\n"); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "feature"); + const updates: CheckoutDiffSnapshotPayload[] = []; + const params = { cwd, compare: { mode: "base" as const, baseRef: "main" } }; + const subscription = await manager.subscribe(params, (snapshot) => updates.push(snapshot)); + try { + expect(subscription.initial.files.map((file) => file.path)).toEqual(["b.ts"]); + git("update-ref", "refs/remotes/origin/main", "HEAD"); + manager.scheduleRefreshForCwd(cwd); + await expect.poll(() => updates.at(-1)?.files.length, { timeout: 5000 }).toBe(0); + expect(updates.at(-1)).toEqual(await manager.read(params)); + } finally { + await subscription.unsubscribe(); + } + }); +}); + +test("cold many-file reads batch Git patches while matching the legacy reader", async () => { + const { getCheckoutDiff } = await import("../utils/checkout-git.js"); + const { startGitCommandMetrics, stopGitCommandMetrics } = + await import("../utils/run-git-command.js"); + await withCheckout(async ({ cwd, manager, git }) => { + const count = 1000; + for (let index = 0; index < count; index++) + await writeFile(join(cwd, `file-${index}.ts`), "export const value = 0;\n"); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "many files"); + for (let index = 0; index < count; index++) + await writeFile(join(cwd, `file-${index}.ts`), "export const value = 1;\n"); + const legacyStart = performance.now(); + const legacy = await getCheckoutDiff(cwd, { mode: "uncommitted", includeStructured: true }); + const legacyMs = performance.now() - legacyStart; + startGitCommandMetrics(); + const streamedStart = performance.now(); + const streamed = await manager.read({ cwd, compare: { mode: "uncommitted" } }); + const streamedMs = performance.now() - streamedStart; + const metrics = stopGitCommandMetrics(); + const patches = metrics.commands.filter((command) => command.args.includes("--patch")); + const artifactDirectory = join(import.meta.dirname, "../../../../.artifacts/diff-subscription"); + await mkdir(artifactDirectory, { recursive: true }); + await writeFile( + join(artifactDirectory, "cold-many-file-metrics.json"), + JSON.stringify({ count, legacyMs, streamedMs, patchCommands: patches.length }, null, 2), + ); + expect(streamed.error).toBeNull(); + expect(streamed.files).toHaveLength(count); + expect(legacy.structured).toHaveLength(count); + expect(streamed.files.map((file) => [file.path, file.hunks])).toEqual( + legacy.structured?.map((file) => [file.path, file.hunks]), + ); + expect(patches.length).toBeLessThanOrEqual(Math.ceil(count / 64)); + }); +}, 60000); diff --git a/packages/server/src/server/checkout-diff-manager.ts b/packages/server/src/server/checkout-diff-manager.ts index 3632f28450..83ea816ba7 100644 --- a/packages/server/src/server/checkout-diff-manager.ts +++ b/packages/server/src/server/checkout-diff-manager.ts @@ -129,6 +129,10 @@ export class CheckoutDiffManager { { cwd, base: { kind: "rev", rev: baseRef }, + checkoutBase: + params.compare.mode === "uncommitted" + ? undefined + : { compare: params.compare, paseoHome: this.paseoHome }, head: params.compare.mode === "base" ? { kind: "rev", rev: "HEAD" } : { kind: "worktree" }, mergeBase: params.compare.mode !== "uncommitted", diff --git a/packages/server/src/server/diff/git.test.ts b/packages/server/src/server/diff/git.test.ts index d06fa2ae03..73f2975c0b 100644 --- a/packages/server/src/server/diff/git.test.ts +++ b/packages/server/src/server/diff/git.test.ts @@ -593,3 +593,38 @@ test("files removed between listing and highlighting retain their deletion patch await rm(cwd, { recursive: true, force: true }); } }); + +test("a batched oversized tracked patch does not hide its small sibling", async () => { + const cwd = await mkdtemp(join(tmpdir(), "diff-batch-budget-")); + const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }); + try { + git("init", "-b", "main"); + git("config", "user.name", "Test"); + git("config", "user.email", "test@example.com"); + await writeFile(join(cwd, "a.txt"), "before\n"); + await writeFile(join(cwd, "b.txt"), "before\n"); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "base"); + await writeFile(join(cwd, "a.txt"), "oversized\n".repeat(1024 * 1024)); + await writeFile(join(cwd, "b.txt"), "after\n"); + const comparison = await listComparison({ + cwd, + base: { kind: "rev", rev: "HEAD" }, + head: { kind: "worktree" }, + highlight: false, + }); + try { + expect(await comparison.read(comparison.files[0])).toEqual({ kind: "too_large" }); + const sibling = await comparison.read(comparison.files[1]); + if (sibling.kind !== "ready") throw new Error("Expected readable sibling"); + expect(sibling.body.hunks.flatMap((hunk) => hunk.lines)).toContainEqual({ + type: "add", + content: "after", + }); + } finally { + await comparison.close(); + } + } finally { + await rm(cwd, { recursive: true, force: true }); + } +}); diff --git a/packages/server/src/server/diff/git.ts b/packages/server/src/server/diff/git.ts index e8bda9bdb7..b655df33e1 100644 --- a/packages/server/src/server/diff/git.ts +++ b/packages/server/src/server/diff/git.ts @@ -1,4 +1,5 @@ import { parseGitRevParsePath } from "../../utils/git-rev-parse-path.js"; +import { Patches } from "./patches.js"; import { HighlightSources } from "./sources.js"; import { prepareAllChangesIndex, @@ -6,6 +7,8 @@ import { GIT_PATCH_FORMAT_ARGS, inspectUntrackedFile, snapshotComparisonIndex, + resolveCheckoutDiffBase, + type CheckoutDiffCompare, } from "../../utils/checkout-git.js"; import { lstat, readFile, readlink } from "node:fs/promises"; import { resolve } from "node:path"; @@ -14,7 +17,9 @@ import type { DiffEntry, DiffBody } from "@getpaseo/protocol/binary-frames/index import { runGitCommand, type GitCommandOptions } from "../../utils/run-git-command.js"; import { parseDiff, highlightDiffWithFileContent } from "../utils/diff-highlighter.js"; -export type Compare = Omit; +export type Compare = Omit & { + checkoutBase?: { compare: CheckoutDiffCompare; paseoHome: string }; +}; export interface ListedFile { entry: DiffEntry; identity: string; @@ -34,6 +39,7 @@ export interface Comparison extends Pick< files: ListedFile[]; metrics: { highlightCalls: number }; close(): Promise; + selectBodies(paths: ReadonlySet): void; read( file: ListedFile, budget?: { remainingBytes?: number; maxPatchBytes?: number }, @@ -125,6 +131,13 @@ function parseStats( return stats; } export async function listComparison(compare: Compare): Promise { + if (compare.checkoutBase) { + const selected = await resolveCheckoutDiffBase(compare.cwd, compare.checkoutBase.compare, { + paseoHome: compare.checkoutBase.paseoHome, + }); + if (!selected) throw new DiffReadError("REF_NOT_FOUND", "Comparison base is unavailable"); + compare = { ...compare, base: { kind: "rev", rev: selected } }; + } validateCompare(compare); const cwd = compare.cwd; let privateIndex: Awaited> = null; @@ -207,9 +220,9 @@ async function readComparisonList( else if (oldPath !== null) change = "renamed"; let identity = header.join(" "); if (compare.head.kind === "worktree" && code !== "D") { - const value = await lstat(resolve(cwd, path), { bigint: true }); - identity += `:${value.size}:${value.mtimeNs}`; + identity += `:${await readWorktreeIdentity(path, header, options)}`; } + files.push({ entry: { path, @@ -227,6 +240,7 @@ async function readComparisonList( if (compare.head.kind === "worktree") files.push(...(await listUntrackedFiles(options))); files.sort((a, b) => a.entry.path.localeCompare(b.entry.path)); const metrics = { highlightCalls: 0 }; + const patches = new Patches(args, options, files); const sources = new HighlightSources({ cwd, baseSha: comparisonBaseSha, @@ -239,32 +253,27 @@ async function readComparisonList( mergeBaseSha, files, metrics, + selectBodies: (paths) => patches.select(paths), close: async () => { await privateIndex?.cleanup(); }, async read(file, budget = {}) { const maxPatchBytes = budget.maxPatchBytes ?? PER_FILE_DIFF_MAX_BYTES; const remainingBytes = budget.remainingBytes ?? maxPatchBytes; - const paths = [file.entry.path]; - if (file.entry.oldPath !== null) paths.unshift(file.entry.oldPath); if (file.source.kind === "untracked" && file.source.size > maxPatchBytes) { return { kind: "too_large" }; } - const command = + const result = file.source.kind === "untracked" - ? ["diff", "--no-index", ...GIT_PATCH_FORMAT_ARGS, "--", "/dev/null", file.entry.path] - : [ - ...args, - ...(file.entry.oldPath !== null ? ["--diff-filter=RC"] : []), - "--patch", - "--", - ...paths.map((path) => `:(literal)${path}`), - ]; - const result = await runGitCommand(command, { - ...options, - acceptExitCodes: [0, 1], - maxOutputBytes: maxPatchBytes + 1, - }); + ? await runGitCommand( + ["diff", "--no-index", ...GIT_PATCH_FORMAT_ARGS, "--", "/dev/null", file.entry.path], + { + ...options, + acceptExitCodes: [0, 1], + maxOutputBytes: maxPatchBytes + 1, + }, + ) + : await patches.read(file, maxPatchBytes); const bytes = Buffer.byteLength(result.stdout); if (result.truncated || bytes > remainingBytes || bytes > maxPatchBytes) return { kind: "too_large" }; @@ -309,6 +318,23 @@ async function readComparisonList( }; } +async function readWorktreeIdentity( + path: string, + header: string[], + options: GitCommandOptions, +): Promise { + const value = await lstat(resolve(options.cwd, path), { bigint: true }); + let identity = `${value.size}:${value.mtimeNs}`; + if (header[0] === ":160000" || header[1] === "160000") { + const submodule = await runGitCommand( + ["status", "--porcelain=v2", "--branch", "--untracked-files=normal"], + { cwd: resolve(options.cwd, path), envOverlay: { GIT_OPTIONAL_LOCKS: "0" } }, + ); + identity += `:${submodule.stdout}`; + } + return identity; +} + function diffArguments(compare: Compare, baseSha: string, headSha: string): string[] { const revisions: string[] = []; if (compare.base.kind !== "index") revisions.push(baseSha); diff --git a/packages/server/src/server/diff/index.ts b/packages/server/src/server/diff/index.ts index 078d12a2e8..06e43a6670 100644 --- a/packages/server/src/server/diff/index.ts +++ b/packages/server/src/server/diff/index.ts @@ -100,6 +100,7 @@ export class DiffSubscriptions { input.signal?.throwIfAborted(); const file = comparison.files.find((candidate) => candidate.entry.path === input.path); if (!file || file.entry.content === "binary") return null; + comparison.selectBodies(new Set([file.entry.path])); const record = await comparison.read(file, { maxPatchBytes: DEFAULT_MAX_OUTPUT_BYTES }); input.signal?.throwIfAborted(); return materializeDiffFile( @@ -122,7 +123,9 @@ export class DiffSubscriptions { }; const key = JSON.stringify([ compare.cwd, - endpointKey(compare.base), + compare.checkoutBase + ? ["checkout", JSON.stringify(compare.checkoutBase)] + : endpointKey(compare.base), endpointKey(compare.head), compare.mergeBase, compare.ignoreWhitespace, @@ -373,6 +376,9 @@ export class DiffSubscriptions { for (const entry of entries) if (entry.content === "text") state.omitted.push(entry.path); return; } + comparison.selectBodies( + new Set(entries.filter((entry) => entry.body).map((entry) => entry.path)), + ); for (let index = 0; index < comparison.files.length; index++) { if (isInterrupted(target)) return; const file = comparison.files[index]; diff --git a/packages/server/src/server/diff/patches.ts b/packages/server/src/server/diff/patches.ts new file mode 100644 index 0000000000..ef5dbc08c1 --- /dev/null +++ b/packages/server/src/server/diff/patches.ts @@ -0,0 +1,80 @@ +import { runGitCommand, type GitCommandOptions } from "../../utils/run-git-command.js"; +import { parseDiff } from "../utils/diff-highlighter.js"; +import type { ListedFile } from "./git.js"; + +interface Patch { + stdout: string; + truncated: boolean; +} + +// Acquire one bounded page lazily. Highlighting and delivery remain per file. +export class Patches { + private files: ListedFile[]; + private pending = new Map>>(); + constructor( + private args: string[], + private options: GitCommandOptions, + files: ListedFile[], + ) { + this.files = files.filter((file) => file.source.kind === "tracked" && file.entry.body); + } + select(paths: ReadonlySet): void { + this.files = this.files.filter((file) => paths.has(file.entry.path)); + } + async read(file: ListedFile, maxBytes: number): Promise { + const existing = this.pending.get(file.entry.path); + if (existing) return (await existing).get(file.entry.path) ?? { stdout: "", truncated: false }; + const renamed = file.entry.oldPath !== null; + const candidates = this.files.filter( + (candidate) => (candidate.entry.oldPath !== null) === renamed, + ); + const index = candidates.indexOf(file); + const page = renamed || index < 0 ? [file] : candidates.slice(index, index + 64); + this.pending.clear(); + const reading = this.load(page, maxBytes); + for (const candidate of page) this.pending.set(candidate.entry.path, reading); + return (await reading).get(file.entry.path) ?? { stdout: "", truncated: false }; + } + + private async load(files: ListedFile[], maxBytes: number): Promise> { + const paths = [ + ...new Set( + files.flatMap((file) => + file.entry.oldPath === null ? [file.entry.path] : [file.entry.oldPath, file.entry.path], + ), + ), + ]; + const result = await runGitCommand( + [ + ...this.args, + ...(files[0].entry.oldPath !== null ? ["--diff-filter=RC"] : []), + "--patch", + "--", + ...paths.map((path) => `:(literal)${path}`), + ], + { + ...this.options, + acceptExitCodes: [0, 1], + maxOutputBytes: (files.length === 1 ? maxBytes : 8 * maxBytes) + 1, + }, + ); + if (files.length === 1) return new Map([[files[0].entry.path, result]]); + if (result.truncated) { + const middle = Math.ceil(files.length / 2); + return new Map([ + ...(await this.load(files.slice(0, middle), maxBytes)), + ...(await this.load(files.slice(middle), maxBytes)), + ]); + } + const patches = new Map(); + for (const section of result.stdout.split(/(?=^diff --git )/m).filter(Boolean)) { + const hunkStart = section.indexOf("\n@@ "); + const parsed = parseDiff(hunkStart < 0 ? section : section.slice(0, hunkStart))[0]; + if (!parsed) throw new Error("Git returned an invalid patch header"); + const previous = patches.get(parsed.path)?.stdout ?? ""; + const stdout = previous + section; + patches.set(parsed.path, { stdout, truncated: Buffer.byteLength(stdout) > maxBytes }); + } + return patches; + } +} diff --git a/packages/server/src/server/diff/subscriptions.test.ts b/packages/server/src/server/diff/subscriptions.test.ts index a96e010c31..71a6fff0b8 100644 --- a/packages/server/src/server/diff/subscriptions.test.ts +++ b/packages/server/src/server/diff/subscriptions.test.ts @@ -359,3 +359,60 @@ test("a cached replay completes before a live refresh can depend on it", async ( await rm(cwd, { recursive: true, force: true }); } }); + +test("submodule commits invalidate cached worktree gitlink bodies", async () => { + const { lstat } = await import("node:fs/promises"); + const cwd = await mkdtemp(join(tmpdir(), "diff-gitlink-cache-")); + const sub = join(cwd, "sub"); + const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); + const streams = new DiffSubscriptions({ + async requestWorkingTreeWatch(root) { + return { repoRoot: root, unsubscribe: async () => {} }; + }, + }); + const frames: DiffFrame[] = []; + try { + git("init", "-b", "main"); + git("config", "user.name", "Test"); + git("config", "user.email", "test@example.com"); + await mkdir(sub); + git("-C", "sub", "init", "-b", "main"); + git("-C", "sub", "config", "user.name", "Test"); + git("-C", "sub", "config", "user.email", "test@example.com"); + await writeFile(join(sub, "a.txt"), "A\n"); + git("-C", "sub", "add", "."); + git("-C", "sub", "-c", "commit.gpgsign=false", "commit", "-m", "A"); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "record A"); + await writeFile(join(sub, "a.txt"), "B\n"); + git("-C", "sub", "-c", "commit.gpgsign=false", "commit", "-am", "B"); + const before = await lstat(sub, { bigint: true }); + const subscription = await streams.subscribe( + { cwd, base: { kind: "rev", rev: "HEAD" }, head: { kind: "worktree" } }, + async (frame) => { + frames.push(frame); + }, + ); + await subscription.start(); + expect(frames.at(-1)).toMatchObject({ opcode: DiffOpcode.End, error: null }); + frames.length = 0; + await writeFile(join(sub, "a.txt"), "C\n"); + git("-C", "sub", "-c", "commit.gpgsign=false", "commit", "-am", "C"); + const sha = git("-C", "sub", "rev-parse", "HEAD"); + expect((await lstat(sub, { bigint: true })).mtimeNs).toBe(before.mtimeNs); + streams.invalidateForCwd(cwd); + await expect + .poll(() => frames.some((frame) => frame.opcode === DiffOpcode.End), { timeout: 5000 }) + .toBe(true); + const bodies = frames.flatMap((frame) => + frame.opcode === DiffOpcode.Bodies ? frame.files : [], + ); + expect(bodies.flatMap((file) => file.hunks.flatMap((hunk) => hunk.lines))).toContainEqual( + expect.objectContaining({ type: "add", content: `Subproject commit ${sha}` }), + ); + await subscription.release(); + } finally { + await streams.close(); + await rm(cwd, { recursive: true, force: true }); + } +});