Skip to content

Detect file renames in review diffs #3916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jakeleventhal wants to merge 5 commits into
base: main
Choose a base branch
from
+266 −59
Open

Detect file renames in review diffs #3916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0ac42fc
Detect file renames in review diffs
jakeleventhal Jul 12, 2026
cd85e36
Preserve sparse checkout metadata in review diffs
jakeleventhal Jul 12, 2026
56ae8a6
Cover staged index-only review changes
jakeleventhal Jul 12, 2026
aa8d932
Include staged files in unborn review diffs
jakeleventhal Jul 12, 2026
0fc42d5
Merge branch 'main' into t3code/detect-file-renames
jakeleventhal Jul 15, 2026
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
168 changes: 168 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,37 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
}),
);

it.effect("includes untracked files before the repository has its first commit", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* driver.initRepo({ cwd });
yield* writeTextFile(cwd, "first.txt", "first\n");

const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false });
const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? "";

assert.include(diff, "diff --git a/first.txt b/first.txt");
assert.include(diff, "+first");
}),
);

it.effect("includes staged files before the repository has its first commit", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* driver.initRepo({ cwd });
yield* writeTextFile(cwd, "staged.txt", "staged\n");
yield* git(cwd, ["add", "staged.txt"]);

const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false });
const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? "";

assert.include(diff, "diff --git a/staged.txt b/staged.txt");
assert.include(diff, "+staged");
}),
);

it.effect("honors whitespace filtering for worktree and branch previews", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
Expand Down Expand Up @@ -243,6 +274,143 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
);
}),
);

it.effect("detects an unstaged rename with edits as one working-tree diff", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const fileSystem = yield* FileSystem.FileSystem;
const pathService = yield* Path.Path;
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* writeTextFile(
cwd,
"before.ts",
[
"export const first = 1;",
"export const second = 2;",
"export const third = 3;",
"export const fourth = 4;",
"",
].join("\n"),
);
yield* git(cwd, ["add", "before.ts"]);
yield* git(cwd, ["commit", "-m", "add source file"]);
yield* fileSystem.rename(
pathService.join(cwd, "before.ts"),
pathService.join(cwd, "after.ts"),
);
yield* writeTextFile(
cwd,
"after.ts",
[
"export const first = 1;",
"export const second = 2;",
"export const third = 30;",
"export const fourth = 4;",
"",
].join("\n"),
);

const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false });
const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? "";

assert.include(diff, "rename from before.ts");
assert.include(diff, "rename to after.ts");
assert.include(diff, "-export const third = 3;");
assert.include(diff, "+export const third = 30;");
assert.notInclude(diff, "--- /dev/null");
assert.notInclude(diff, "+++ /dev/null");
}),
);

it.effect("preserves sparse-checkout entries in the working-tree diff", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* writeTextFile(cwd, "included/file.ts", "export const included = 1;\n");
yield* writeTextFile(cwd, "excluded/file.ts", "export const excluded = 1;\n");
yield* git(cwd, ["add", "."]);
yield* git(cwd, ["commit", "-m", "add sparse files"]);
yield* git(cwd, ["sparse-checkout", "set", "included"]);
yield* writeTextFile(cwd, "included/file.ts", "export const included = 2;\n");

const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false });
const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? "";

assert.include(diff, "diff --git a/included/file.ts b/included/file.ts");
assert.include(diff, "+export const included = 2;");
assert.notInclude(diff, "excluded/file.ts");
}),
);

it.effect("includes staged index-only deletions in the working-tree diff", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* writeTextFile(cwd, ".gitignore", "secret.env\n");
yield* writeTextFile(cwd, "secret.env", "SECRET=value\n");
yield* git(cwd, ["add", "-f", ".gitignore", "secret.env"]);
yield* git(cwd, ["commit", "-m", "add ignored tracked file"]);
yield* git(cwd, ["rm", "--cached", "secret.env"]);

const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false });
const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? "";

assert.include(diff, "diff --git a/secret.env b/secret.env");
assert.include(diff, "deleted file mode");
assert.include(diff, "-SECRET=value");
}),
);

it.effect("detects a committed rename with edits in the branch diff", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const { initialBranch } = yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* writeTextFile(
cwd,
"before.ts",
[
"export const first = 1;",
"export const second = 2;",
"export const third = 3;",
"export const fourth = 4;",
"",
].join("\n"),
);
yield* git(cwd, ["add", "before.ts"]);
yield* git(cwd, ["commit", "-m", "add source file"]);
yield* git(cwd, ["checkout", "-b", "feature/rename"]);
yield* git(cwd, ["mv", "before.ts", "after.ts"]);
yield* writeTextFile(
cwd,
"after.ts",
[
"export const first = 1;",
"export const second = 2;",
"export const third = 30;",
"export const fourth = 4;",
"",
].join("\n"),
);
yield* git(cwd, ["add", "after.ts"]);
yield* git(cwd, ["commit", "-m", "rename source file"]);

const preview = yield* driver.getReviewDiffPreview({
cwd,
baseRef: initialBranch,
ignoreWhitespace: false,
});
const diff = preview.sources.find((source) => source.kind === "branch-range")?.diff ?? "";

assert.include(diff, "rename from before.ts");
assert.include(diff, "rename to after.ts");
assert.include(diff, "-export const third = 3;");
assert.include(diff, "+export const third = 30;");
}),
);
});

describe("repository status", () => {
Expand Down
157 changes: 98 additions & 59 deletions apps/server/src/vcs/GitVcsDriverCore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ const RANGE_COMMIT_SUMMARY_MAX_OUTPUT_BYTES = 19_000;
const RANGE_DIFF_SUMMARY_MAX_OUTPUT_BYTES = 19_000;
const RANGE_DIFF_PATCH_MAX_OUTPUT_BYTES = 59_000;
const REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES = 120_000;
const REVIEW_UNTRACKED_DIFF_MAX_OUTPUT_BYTES = 80_000;
const WORKSPACE_FILES_MAX_OUTPUT_BYTES = 120_000;
const STATUS_UPSTREAM_REFRESH_INTERVAL = Duration.seconds(15);
const STATUS_UPSTREAM_REFRESH_TIMEOUT = Duration.seconds(5);

Expand Down Expand Up @@ -1807,43 +1805,103 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
};
});

const readUntrackedReviewDiffs = Effect.fn("readUntrackedReviewDiffs")(function* (cwd: string) {
const untrackedResult = yield* executeGit(
"GitVcsDriver.readUntrackedReviewDiffs.list",
cwd,
["ls-files", "--others", "--exclude-standard", "-z"],
{
maxOutputBytes: WORKSPACE_FILES_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
);
const untrackedPaths = splitNullSeparatedGitStdoutPaths(untrackedResult);
if (untrackedPaths.length === 0) {
return { diff: "", truncated: untrackedResult.stdoutTruncated };
}
const readWorkingTreeReviewDiff = Effect.fn("readWorkingTreeReviewDiff")(function* (
cwd: string,
ignoreWhitespace: boolean | undefined,
) {
const tempDirectory = yield* fileSystem
.makeTempDirectory({ prefix: "t3code-review-index-" })
.pipe(
Effect.mapError(
(cause) =>
new GitCommandError({
operation: "GitVcsDriver.readWorkingTreeReviewDiff.createTempIndex",
command: "git diff",
cwd,
detail: "Failed to create a temporary Git index for the review diff.",
cause,
}),
),
);
const indexPath = path.join(tempDirectory, "index");
const env = { GIT_INDEX_FILE: indexPath } satisfies NodeJS.ProcessEnv;

return yield* Effect.gen(function* () {
const headResult = yield* executeGit(
"GitVcsDriver.readWorkingTreeReviewDiff.resolveHead",
cwd,
["rev-parse", "--verify", "--quiet", "HEAD"],
{ allowNonZeroExit: true },
);
const hasHead = headResult.exitCode === 0;
const baseline = hasHead
? "HEAD"
: (yield* executeGit(
"GitVcsDriver.readWorkingTreeReviewDiff.resolveEmptyTree",
cwd,
["hash-object", "-t", "tree", "--stdin"],
{ stdin: "" },
)).stdout.trim();
const gitIndexResult = yield* executeGit(
"GitVcsDriver.readWorkingTreeReviewDiff.resolveIndex",
cwd,
["rev-parse", "--git-path", "index"],
);
const gitIndexPath = path.isAbsolute(gitIndexResult.stdout.trim())
? gitIndexResult.stdout.trim()
: path.resolve(cwd, gitIndexResult.stdout.trim());

const diffs = yield* Effect.forEach(
untrackedPaths,
(relativePath) =>
executeGit(
"GitVcsDriver.readUntrackedReviewDiffs.diff",
if (yield* fileSystem.exists(gitIndexPath)) {
yield* fileSystem.copyFile(gitIndexPath, indexPath).pipe(
Effect.mapError(
(cause) =>
new GitCommandError({
operation: "GitVcsDriver.readWorkingTreeReviewDiff.copyIndex",
command: "git diff",
cwd,
detail: "Failed to copy the Git index for the review diff.",
cause,
}),
),
);
} else {
yield* executeGit(
"GitVcsDriver.readWorkingTreeReviewDiff.readEmptyTree",
cwd,
["diff", "--no-index", "--patch", "--minimal", "--", "/dev/null", relativePath],
{
allowNonZeroExit: true,
maxOutputBytes: REVIEW_UNTRACKED_DIFF_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
),
{ concurrency: 4 },
);
["read-tree", "--empty"],
{ env },
);
}
yield* executeGit(
"GitVcsDriver.readWorkingTreeReviewDiff.add",
cwd,
["add", "--intent-to-add", "-A", "--", "."],
Comment thread
jakeleventhal marked this conversation as resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve non-ignored index-only deletions

Here git add -h says -A adds changes from tracked and untracked files and -N records only the fact a path will be added, so this temp-index refresh no longer preserves an existing index-only deletion when the same path remains as a non-ignored untracked file. Fresh evidence beyond the prior ignored-file case: after git rm --cached f without adding f to .gitignore, this command turns the copied index into DA f, and the following git diff HEAD is empty if the file still matches HEAD (or shows a modification if it changed), whereas the real index represents a deletion to be committed; the review preview can therefore miss or misstate files being untracked.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep tracked diffs when untracked repos cannot be added

Because git add -h confirms -A scans untracked files too, this command can fail before the diff is generated for an otherwise valid dirty worktree; for example, an untracked nested Git repository with no commit makes git add --intent-to-add -A -- . exit 128 (does not have a commit checked out). Since the caller converts any readWorkingTreeReviewDiff failure into an empty working-tree preview, the presence of that scratch nested repo hides unrelated tracked modifications that the old tracked git diff HEAD -- path still reported.

Useful? React with 👍 / 👎.

{ env },
);

@cursor cursor Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add -A erases index-only changes

High Severity

The readWorkingTreeReviewDiff function's git add --intent-to-add -A command on the temporary index re-adds files that were git rm --cached but still exist in the working tree. This causes the working-tree review diff to incorrectly omit staged deletions and other staged-only index changes.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0fc42d5. Configure here.


return {
diff: Arr.filterMap(diffs, (result) =>
result.stdout.trim().length > 0 ? Result.succeed(result.stdout) : Result.failVoid,
).join("\n"),
truncated: untrackedResult.stdoutTruncated || diffs.some((result) => result.stdoutTruncated),
};
return yield* executeGit(
"GitVcsDriver.readWorkingTreeReviewDiff.diff",
cwd,
[
"diff",
"--patch",
"--minimal",
"--find-renames",
...(ignoreWhitespace ? ["--ignore-all-space"] : []),
baseline,
"--",
],
{
env,
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
);
}).pipe(
Effect.ensuring(
fileSystem.remove(tempDirectory, { recursive: true, force: true }).pipe(Effect.ignore),
),
);
});

const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* (
Expand All @@ -1867,22 +1925,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
)
: null);

const dirtyTrackedResult = yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.dirtyTracked",
input.cwd,
[
"diff",
"--patch",
"--minimal",
...(input.ignoreWhitespace ? ["--ignore-all-space"] : []),
"HEAD",
"--",
],
{
maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
appendTruncationMarker: true,
},
).pipe(
const dirtyResult = yield* readWorkingTreeReviewDiff(input.cwd, input.ignoreWhitespace).pipe(
Effect.orElseSucceed(() => ({
exitCode: 0,
stdout: "",
Expand All @@ -1891,12 +1934,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
stderrTruncated: false,
})),
);
const dirtyUntracked = yield* readUntrackedReviewDiffs(input.cwd).pipe(
Effect.orElseSucceed(() => ({ diff: "", truncated: false })),
);
const dirtyDiff = [dirtyTrackedResult.stdout.trimEnd(), dirtyUntracked.diff.trimEnd()]
.filter((diff) => diff.length > 0)
.join("\n");
const dirtyDiff = dirtyResult.stdout;

const baseResult =
baseRef && branch
Expand All @@ -1907,6 +1945,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
"diff",
"--patch",
"--minimal",
"--find-renames",
...(input.ignoreWhitespace ? ["--ignore-all-space"] : []),
`${baseRef}...HEAD`,
],
Expand Down Expand Up @@ -1953,7 +1992,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
headRef: null,
diff: dirtyDiff,
diffHash: dirtyDiffHash,
truncated: dirtyTrackedResult.stdoutTruncated || dirtyUntracked.truncated,
truncated: dirtyResult.stdoutTruncated,
},
{
id: "branch-range",
Expand Down
Loading