diff --git a/src/workspace/get-workspace-changes.ts b/src/workspace/get-workspace-changes.ts index cef67289c..a67795233 100644 --- a/src/workspace/get-workspace-changes.ts +++ b/src/workspace/get-workspace-changes.ts @@ -38,6 +38,7 @@ interface ChangesFromRefInput { interface DiffStat { additions: number; deletions: number; + isBinary: boolean; } interface FileFingerprint { @@ -192,13 +193,13 @@ async function readWorkingTreeFile(repoRoot: string, path: string): Promise line.trim()) + .find(Boolean); + if (!firstLine) { + return null; + } + const [addedRaw, deletedRaw] = firstLine.split("\t"); + if (addedRaw === "-" && deletedRaw === "-") { + return { + additions: 0, + deletions: 0, + isBinary: true, + }; + } + const additions = Number.parseInt(addedRaw ?? "", 10); + const deletions = Number.parseInt(deletedRaw ?? "", 10); + return { + additions: Number.isFinite(additions) ? additions : 0, + deletions: Number.isFinite(deletions) ? deletions : 0, + isBinary: false, }; } async function readDiffStat(repoRoot: string, path: string): Promise { try { const output = await getGitStdout(["diff", "--numstat", "HEAD", "--", path], repoRoot); - const firstLine = output - .split("\n") - .map((line) => line.trim()) - .find(Boolean); - if (!firstLine) { - return null; - } - const [addedRaw, deletedRaw] = firstLine.split("\t"); - const additions = Number.parseInt(addedRaw ?? "", 10); - const deletions = Number.parseInt(deletedRaw ?? "", 10); - return { - additions: Number.isFinite(additions) ? additions : 0, - deletions: Number.isFinite(deletions) ? deletions : 0, - }; + return parseDiffStatOutput(output); } catch { return null; } @@ -239,20 +253,7 @@ async function readDiffStatBetweenRefs( ): Promise { try { const output = await getGitStdout(["diff", "--numstat", fromRef, toRef, "--", path], repoRoot); - const firstLine = output - .split("\n") - .map((line) => line.trim()) - .find(Boolean); - if (!firstLine) { - return null; - } - const [addedRaw, deletedRaw] = firstLine.split("\t"); - const additions = Number.parseInt(addedRaw ?? "", 10); - const deletions = Number.parseInt(deletedRaw ?? "", 10); - return { - additions: Number.isFinite(additions) ? additions : 0, - deletions: Number.isFinite(deletions) ? deletions : 0, - }; + return parseDiffStatOutput(output); } catch { return null; } @@ -261,20 +262,7 @@ async function readDiffStatBetweenRefs( async function readDiffStatFromRef(repoRoot: string, fromRef: string, path: string): Promise { try { const output = await getGitStdout(["diff", "--numstat", fromRef, "--", path], repoRoot); - const firstLine = output - .split("\n") - .map((line) => line.trim()) - .find(Boolean); - if (!firstLine) { - return null; - } - const [addedRaw, deletedRaw] = firstLine.split("\t"); - const additions = Number.parseInt(addedRaw ?? "", 10); - const deletions = Number.parseInt(deletedRaw ?? "", 10); - return { - additions: Number.isFinite(additions) ? additions : 0, - deletions: Number.isFinite(deletions) ? deletions : 0, - }; + return parseDiffStatOutput(output); } catch { return null; } @@ -282,13 +270,17 @@ async function readDiffStatFromRef(repoRoot: string, fromRef: string, path: stri async function buildFileChange(repoRoot: string, entry: NameStatusEntry): Promise { const basePath = entry.previousPath ?? entry.path; + const diffStat = entry.status === "untracked" ? null : await readDiffStat(repoRoot, entry.path); const oldText = - entry.status === "added" || entry.status === "untracked" ? null : await readHeadFile(repoRoot, basePath); - const newText = entry.status === "deleted" ? null : await readWorkingTreeFile(repoRoot, entry.path); + entry.status === "added" || entry.status === "untracked" || diffStat?.isBinary + ? null + : await readHeadFile(repoRoot, basePath); + const newText = + entry.status === "deleted" || diffStat?.isBinary ? null : await readWorkingTreeFile(repoRoot, entry.path); const stats = entry.status === "untracked" ? { additions: toLineCount(newText ?? ""), deletions: 0 } - : ((await readDiffStat(repoRoot, entry.path)) ?? fallbackStats(oldText, newText)); + : (diffStat ?? fallbackStats(oldText, newText)); return { path: entry.path, @@ -308,10 +300,12 @@ async function buildFileChangeBetweenRefs( toRef: string, ): Promise { const basePath = entry.previousPath ?? entry.path; - const oldText = entry.status === "added" ? null : await readFileAtRef(repoRoot, fromRef, basePath); - const newText = entry.status === "deleted" ? null : await readFileAtRef(repoRoot, toRef, entry.path); - const stats = - (await readDiffStatBetweenRefs(repoRoot, fromRef, toRef, entry.path)) ?? fallbackStats(oldText, newText); + const diffStat = await readDiffStatBetweenRefs(repoRoot, fromRef, toRef, entry.path); + const oldText = + entry.status === "added" || diffStat?.isBinary ? null : await readFileAtRef(repoRoot, fromRef, basePath); + const newText = + entry.status === "deleted" || diffStat?.isBinary ? null : await readFileAtRef(repoRoot, toRef, entry.path); + const stats = diffStat ?? fallbackStats(oldText, newText); return { path: entry.path, @@ -330,15 +324,17 @@ async function buildFileChangeFromRef( fromRef: string, ): Promise { const basePath = entry.previousPath ?? entry.path; + const diffStat = entry.status === "untracked" ? null : await readDiffStatFromRef(repoRoot, fromRef, entry.path); const oldText = - entry.status === "added" || entry.status === "untracked" + entry.status === "added" || entry.status === "untracked" || diffStat?.isBinary ? null : await readFileAtRef(repoRoot, fromRef, basePath); - const newText = entry.status === "deleted" ? null : await readWorkingTreeFile(repoRoot, entry.path); + const newText = + entry.status === "deleted" || diffStat?.isBinary ? null : await readWorkingTreeFile(repoRoot, entry.path); const stats = entry.status === "untracked" ? { additions: toLineCount(newText ?? ""), deletions: 0 } - : ((await readDiffStatFromRef(repoRoot, fromRef, entry.path)) ?? fallbackStats(oldText, newText)); + : (diffStat ?? fallbackStats(oldText, newText)); return { path: entry.path, diff --git a/test/runtime/get-workspace-changes.test.ts b/test/runtime/get-workspace-changes.test.ts new file mode 100644 index 000000000..6e1e7dd02 --- /dev/null +++ b/test/runtime/get-workspace-changes.test.ts @@ -0,0 +1,92 @@ +import { execFile } from "node:child_process"; +import { mkdtemp, open, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { promisify } from "node:util"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { getWorkspaceChanges, getWorkspaceChangesBetweenRefs } from "../../src/workspace/get-workspace-changes"; + +const execFileAsync = promisify(execFile); +const FIFTY_MIB = 50 * 1024 * 1024; + +async function runGit(repoRoot: string, args: string[]): Promise { + const { stdout } = await execFileAsync("git", ["-c", "core.quotepath=false", ...args], { + cwd: repoRoot, + encoding: "utf8", + env: { + ...process.env, + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_CONFIG_NOSYSTEM: "1", + }, + }); + return stdout.trim(); +} + +async function commitAll(repoRoot: string, message: string): Promise { + await runGit(repoRoot, ["add", "."]); + await runGit(repoRoot, ["commit", "-m", message]); + return await runGit(repoRoot, ["rev-parse", "HEAD"]); +} + +async function createSparseFile(path: string, size: number): Promise { + const handle = await open(path, "w"); + try { + await handle.truncate(size); + } finally { + await handle.close(); + } +} + +describe("getWorkspaceChanges", () => { + let repoRoot: string; + + beforeEach(async () => { + repoRoot = await mkdtemp(join(tmpdir(), "kanban-workspace-changes-")); + await runGit(repoRoot, ["init"]); + await runGit(repoRoot, ["config", "user.email", "test@example.com"]); + await runGit(repoRoot, ["config", "user.name", "Test User"]); + }); + + afterEach(async () => { + await rm(repoRoot, { recursive: true, force: true }); + }); + + it("skips a 50 MiB working tree binary without loading file text", async () => { + const filePath = join(repoRoot, "large.bin"); + await writeFile(filePath, "initial\n"); + await commitAll(repoRoot, "Initial commit"); + + await createSparseFile(filePath, FIFTY_MIB); + + const response = await getWorkspaceChanges(repoRoot); + const file = response.files.find((entry) => entry.path === "large.bin"); + + expect(file).toBeDefined(); + expect(file?.status).toBe("modified"); + expect(file?.oldText).toBeNull(); + expect(file?.newText).toBeNull(); + }); + + it("skips a 50 MiB historical binary before reading it from git", async () => { + const filePath = join(repoRoot, "large.bin"); + await writeFile(filePath, "initial\n"); + const smallRef = await commitAll(repoRoot, "Initial commit"); + + await createSparseFile(filePath, FIFTY_MIB); + const largeRef = await commitAll(repoRoot, "Large binary commit"); + + const response = await getWorkspaceChangesBetweenRefs({ + cwd: repoRoot, + fromRef: smallRef, + toRef: largeRef, + }); + const file = response.files.find((entry) => entry.path === "large.bin"); + + expect(file).toBeDefined(); + expect(file?.status).toBe("modified"); + expect(file?.oldText).toBeNull(); + expect(file?.newText).toBeNull(); + }); +}); diff --git a/web-ui/src/components/detail-panels/diff-viewer-panel.test.tsx b/web-ui/src/components/detail-panels/diff-viewer-panel.test.tsx index b8c538f5b..53a0dd60b 100644 --- a/web-ui/src/components/detail-panels/diff-viewer-panel.test.tsx +++ b/web-ui/src/components/detail-panels/diff-viewer-panel.test.tsx @@ -265,6 +265,7 @@ describe("DiffViewerPanel", () => { expect(container.textContent).toContain("assets/logo.png"); expect(container.textContent).toContain("Binary"); + expect(container.textContent).toContain("Binary file not shown."); expect(container.querySelector(".kb-diff-row")).toBeNull(); }); diff --git a/web-ui/src/components/detail-panels/diff-viewer-panel.tsx b/web-ui/src/components/detail-panels/diff-viewer-panel.tsx index 5c9b8f0e3..8fa628105 100644 --- a/web-ui/src/components/detail-panels/diff-viewer-panel.tsx +++ b/web-ui/src/components/detail-panels/diff-viewer-panel.tsx @@ -889,7 +889,17 @@ export function DiffViewerPanel({ > {group.entries.map((entry) => (
- {entry.isBinary ? null : viewMode === "split" ? ( + {entry.isBinary ? ( +
+ Binary file not shown. +
+ ) : viewMode === "split" ? ( { expect(scrollContainer.scrollTop).toBe(547); }); - it("shows only the file header for binary paths", async () => { + it("shows a binary not shown message for binary paths", async () => { const diffSource: GitCommitDiffSource = { type: "commit", files: [ @@ -162,6 +162,7 @@ describe("GitCommitDiffPanel", () => { expect(container.textContent).toContain("assets/logo.png"); expect(container.textContent).toContain("Binary"); + expect(container.textContent).toContain("Binary file not shown."); expect(container.textContent).not.toContain("No textual diff available."); expect(container.querySelector(".kb-diff-row")).toBeNull(); }); diff --git a/web-ui/src/components/git-history/git-commit-diff-panel.tsx b/web-ui/src/components/git-history/git-commit-diff-panel.tsx index a9124f356..5d2b24a29 100644 --- a/web-ui/src/components/git-history/git-commit-diff-panel.tsx +++ b/web-ui/src/components/git-history/git-commit-diff-panel.tsx @@ -412,9 +412,19 @@ export function GitCommitDiffPanel({ Renamed from {commitFile.previousPath}
) : null} - {!isBinaryFile && rows.length > 0 ? ( + {isBinaryFile ? ( +
+ Binary file not shown. +
+ ) : rows.length > 0 ? ( - ) : !isBinaryFile ? ( + ) : (
No textual diff available.
- ) : null} + )} ) : null}