Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions apps/pi-extension/server/vcs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { spawn, spawnSync } from "node:child_process";
import { readFileSync } from "node:fs";
import { lstatSync, readFileSync, readlinkSync } from "node:fs";
import { resolve as resolvePath } from "node:path";
import {
type DiffResult,
type DiffType,
Expand Down Expand Up @@ -42,7 +43,7 @@ function runCommand(
cwd: options?.cwd,
detached: isolateProcessGroup,
env: preparedGitCommand?.env ?? commandEnvironment,
stdio: ["ignore", "pipe", "pipe"],
stdio: [options?.stdin === undefined ? "ignore" : "pipe", "pipe", "pipe"],
windowsHide: true,
});

Expand Down Expand Up @@ -73,6 +74,7 @@ function runCommand(
const stderrChunks: Buffer[] = [];
proc.stdout!.on("data", (chunk: Buffer) => stdoutChunks.push(chunk));
proc.stderr!.on("data", (chunk: Buffer) => stderrChunks.push(chunk));
if (options?.stdin !== undefined) proc.stdin!.end(options.stdin);

proc.on("close", (code) => {
if (timer) clearTimeout(timer);
Expand Down Expand Up @@ -106,6 +108,31 @@ export const reviewRuntime: ReviewGitRuntime = {
return null;
}
},

async getFileInfo(basePath, path) {
const fullPath = resolvePath(basePath ?? "", path);
try {
const fileStat = lstatSync(fullPath);
return {
path: fullPath,
size: fileStat.size,
mtimeMs: fileStat.mtimeMs,
isFile: fileStat.isFile(),
isSymbolicLink: fileStat.isSymbolicLink(),
isExecutable: (fileStat.mode & 0o111) !== 0,
};
} catch {
return null;
}
},

async readLink(path: string): Promise<string | null> {
try {
return readlinkSync(path);
} catch {
return null;
}
},
};

export const jjRuntime: ReviewJjRuntime = {
Expand Down
40 changes: 39 additions & 1 deletion packages/server/git-background.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { afterEach, describe, expect, test } from "bun:test";
import { spawnSync } from "node:child_process";
import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import {
chmodSync,
mkdirSync,
mkdtempSync,
readFileSync,
rmSync,
symlinkSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { pathToFileURL } from "node:url";
Expand Down Expand Up @@ -98,6 +106,36 @@ process.exit(1);
};
}

describe.skipIf(process.platform === "win32")("review runtime filesystem seam", () => {
for (const fixture of fixtures) {
test(`${fixture.name} resolves file metadata and symlink payloads`, async () => {
const root = mkdtempSync(join(tmpdir(), "plannotator-runtime-file-"));
tempDirs.push(root);
const file = join(root, "file.txt");
const link = join(root, "file-link");
writeFileSync(file, "content\n", "utf-8");
symlinkSync("file.txt", link);

const runtimeModule = await import(pathToFileURL(fixture.modulePath).href);
const runtime = runtimeModule[fixture.exportName] as {
getFileInfo(basePath: string, path: string): Promise<{
path: string;
size: number;
isFile: boolean;
isSymbolicLink: boolean;
} | null>;
readLink(path: string): Promise<string | null>;
};
const fileInfo = await runtime.getFileInfo(root, "file.txt");
const linkInfo = await runtime.getFileInfo(root, "file-link");

expect(fileInfo).toMatchObject({ path: file, size: 8, isFile: true, isSymbolicLink: false });
expect(linkInfo).toMatchObject({ path: link, isFile: false, isSymbolicLink: true });
await expect(runtime.readLink(link)).resolves.toBe("file.txt");
});
}
});

function createHttpCredentialFixture(remoteUrl: string): {
repo: string;
askpassMarker: string;
Expand Down
30 changes: 29 additions & 1 deletion packages/server/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* Used by both Claude Code hook and OpenCode plugin.
*/

import { lstat, readlink } from "node:fs/promises";
import { resolve as resolvePath } from "node:path";

import {
type DiffOption,
type DiffResult,
Expand Down Expand Up @@ -47,7 +50,9 @@ async function runGit(
cwd: options?.cwd,
detached: command.isolateProcessGroup,
env: command.env,
stdin: "ignore",
stdin: options?.stdin === undefined
? "ignore"
: new TextEncoder().encode(options.stdin),
stdout: "pipe",
stderr: "pipe",
windowsHide: true,
Expand Down Expand Up @@ -96,6 +101,29 @@ export const runtime: ReviewGitRuntime = {
return null;
}
},
async getFileInfo(basePath, path) {
const fullPath = resolvePath(basePath ?? "", path);
try {
const fileStat = await lstat(fullPath);
return {
path: fullPath,
size: fileStat.size,
mtimeMs: fileStat.mtimeMs,
isFile: fileStat.isFile(),
isSymbolicLink: fileStat.isSymbolicLink(),
isExecutable: (fileStat.mode & 0o111) !== 0,
};
} catch {
return null;
}
},
async readLink(path: string): Promise<string | null> {
try {
return await readlink(path);
} catch {
return null;
}
},
};

export function getCurrentBranch(): Promise<string> {
Expand Down
6 changes: 6 additions & 0 deletions packages/shared/commit-history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ function git(cwd: string, args: string[]): string {

function makeRuntime(baseCwd: string): ReviewGitRuntime {
return {
async getFileInfo() {
return null;
},
async readLink() {
return null;
},
async runGit(args: string[], options?: { cwd?: string }) {
const result = spawnSync("git", args, {
cwd: options?.cwd ?? baseCwd,
Expand Down
30 changes: 28 additions & 2 deletions packages/shared/diff-fingerprint.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { afterAll, beforeAll, describe, expect, test } from "bun:test";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { lstatSync, mkdtempSync, readlinkSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { join, resolve as resolvePath } from "node:path";
import {
getGitDiffFingerprint,
MAX_REVIEW_FILE_CONTENT_BYTES,
Expand All @@ -16,6 +16,9 @@ const runtime: ReviewGitRuntime = {
cwd: options?.cwd,
stdout: "pipe",
stderr: "pipe",
stdin: options?.stdin === undefined
? "ignore"
: new TextEncoder().encode(options.stdin),
});
const [stdout, stderr] = await Promise.all([
new Response(proc.stdout).text(),
Expand All @@ -31,6 +34,29 @@ const runtime: ReviewGitRuntime = {
return null;
}
},
async getFileInfo(basePath, path) {
const fullPath = resolvePath(basePath ?? "", path);
try {
const fileStat = lstatSync(fullPath);
return {
path: fullPath,
size: fileStat.size,
mtimeMs: fileStat.mtimeMs,
isFile: fileStat.isFile(),
isSymbolicLink: fileStat.isSymbolicLink(),
isExecutable: (fileStat.mode & 0o111) !== 0,
};
} catch {
return null;
}
},
async readLink(path) {
try {
return readlinkSync(path);
} catch {
return null;
}
},
};

let repo: string;
Expand Down
96 changes: 95 additions & 1 deletion packages/shared/gitbutler-core.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, test } from "bun:test";

import type { GitCommandOptions, GitCommandResult } from "./review-core";
import { MAX_REVIEW_FILE_CONTENT_BYTES } from "./review-core";
import {
GITBUTLER_WORKSPACE_DIFF,
GitButlerContractError,
Expand All @@ -25,6 +26,8 @@ const ROOT = "/repo";
const MERGE_BASE = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
const LOWER_TIP = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
const TOP_TIP = "cccccccccccccccccccccccccccccccccccccccc";
const PATCH_OLD_OBJECT = "d".repeat(40);
const PATCH_NEW_OBJECT = "e".repeat(40);

function commandResult(
stdout = "",
Expand Down Expand Up @@ -87,7 +90,13 @@ function createRuntime(options: {
let patch = "diff --git a/file.txt b/file.txt\n-old\n+new\n";

const runtime: ReviewGitButlerRuntime = {
async runGit(args: string[]): Promise<GitCommandResult> {
async getFileInfo() {
return null;
},
async readLink() {
return null;
},
async runGit(args: string[], commandOptions?: GitCommandOptions): Promise<GitCommandResult> {
gitCalls.push(args);
const commandArgs = args[0] === "--no-optional-locks" ? args.slice(1) : args;
if (commandArgs[0] === "symbolic-ref") {
Expand Down Expand Up @@ -116,6 +125,21 @@ function createRuntime(options: {
if (commandArgs[0] === "merge-base") {
return commandResult(`${MERGE_BASE}\n`);
}
if (commandArgs[0] === "cat-file" && commandArgs[1] === "-s") {
return commandResult("10\n");
}
if (commandArgs[0] === "cat-file" && commandArgs.some((arg) => arg.startsWith("--batch-check"))) {
return commandResult(
(commandOptions?.stdin ?? "").trim().split("\n").filter(Boolean).map((objectId) =>
`${objectId} blob 10`,
).join("\n"),
);
}
if (commandArgs[0] === "diff" && commandArgs.includes("--raw")) {
return commandResult(
`:100644 100644 ${PATCH_OLD_OBJECT} ${PATCH_NEW_OBJECT} M\0file.txt\0`,
);
}
if (commandArgs[0] === "diff") return commandResult(patch);
if (commandArgs[0] === "status") return commandResult();
if (commandArgs[0] === "ls-files") return commandResult();
Expand Down Expand Up @@ -542,6 +566,54 @@ describe("GitButler diffs and expansion", () => {
)).resolves.toMatchObject({ patch });
});

test("omits oversized committed object diffs with a content-sensitive binary stub", async () => {
const fixture = createRuntime();
const originalRunGit = fixture.runtime.runGit.bind(fixture.runtime);
const oldObjectId = "d".repeat(40);
let newObjectId = "e".repeat(40);
let sawRawDiff = false;
fixture.setPatch("x".repeat(MAX_REVIEW_FILE_CONTENT_BYTES + 1));
fixture.runtime.runGit = async (args, options) => {
const commandArgs = args[0] === "-c" ? args.slice(2) : args;
if (commandArgs[0] === "diff" && commandArgs.includes("--raw")) {
sawRawDiff = true;
return commandResult(
`:100644 100644 ${oldObjectId} ${newObjectId} M\0large [*]?.txt\0`,
);
}
if (commandArgs[0] === "cat-file") {
return commandResult(`${MAX_REVIEW_FILE_CONTENT_BYTES + 1}\n`);
}
if (commandArgs[0] === "diff" && commandArgs.some((arg) => arg.startsWith(":(top,exclude,literal)"))) {
return commandResult();
}
return originalRunGit(args, options);
};

const result = await runGitButlerDiff(
fixture.runtime,
"gitbutler:branch:feature%2Ftop%20lane",
ROOT,
);
expect(result.patch.length).toBeLessThan(2_000);
expect(result.patch).toContain("Binary files");
expect(result.patch).not.toContain("xxxxxxxxxx");
expect(sawRawDiff).toBe(true);

const first = await getGitButlerDiffFingerprint(
fixture.runtime,
"gitbutler:branch:feature%2Ftop%20lane",
ROOT,
);
newObjectId = "f".repeat(40);
const second = await getGitButlerDiffFingerprint(
fixture.runtime,
"gitbutler:branch:feature%2Ftop%20lane",
ROOT,
);
expect(second).not.toBe(first);
});

test("returns explicit errors when status or a selected target disappears", async () => {
const failedStatus = createRuntime({ status: commandResult("", "database locked", 1) });
await expect(runGitButlerDiff(failedStatus.runtime, GITBUTLER_WORKSPACE_DIFF, ROOT)).resolves.toMatchObject({
Expand Down Expand Up @@ -574,6 +646,28 @@ describe("GitButler diffs and expansion", () => {
});
});

test("does not expand oversized committed GitButler blobs", async () => {
const fixture = createRuntime();
const originalRunGit = fixture.runtime.runGit.bind(fixture.runtime);
let showCalls = 0;
fixture.runtime.runGit = async (args, options) => {
if (args[0] === "cat-file" && args[1] === "-s") {
return commandResult(`${MAX_REVIEW_FILE_CONTENT_BYTES + 1}\n`);
}
if (args[0] === "show") showCalls++;
return originalRunGit(args, options);
};

await expect(getGitButlerFileContentsForDiff(
fixture.runtime,
"gitbutler:branch:feature%2Ftop%20lane",
"src/new.ts",
"src/old.ts",
ROOT,
)).resolves.toEqual({ oldContent: null, newContent: null });
expect(showCalls).toBe(0);
});

test("fingerprints the exact visible patch content", async () => {
const fixture = createRuntime();
const first = await getGitButlerDiffFingerprint(
Expand Down
Loading