Skip to content

Commit 74b725b

Browse files
authored
fix(git): avoid failing on untracked directory symlinks (#169)
1 parent 7be4d17 commit 74b725b

4 files changed

Lines changed: 74 additions & 5 deletions

File tree

src/core/git.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import fs from "node:fs";
2+
import { join } from "node:path";
13
import { HunkUserError } from "./errors";
24
import type { GitCommandInput, ShowCommandInput, StashShowCommandInput } from "./types";
35

@@ -306,10 +308,45 @@ function parseUntrackedFilePaths(statusText: string) {
306308
.flatMap((entry) => (entry.startsWith("?? ") ? [entry.slice(3)] : []));
307309
}
308310

311+
/** Return whether one untracked path can be synthesized into a file diff. */
312+
function isReviewableUntrackedPath(repoRoot: string, filePath: string) {
313+
const absolutePath = join(repoRoot, filePath);
314+
315+
let pathInfo: fs.Stats;
316+
try {
317+
pathInfo = fs.lstatSync(absolutePath);
318+
} catch {
319+
// If the path disappeared after `git status`, let the downstream Git diff
320+
// surface the same error path users would have seen before this filter.
321+
return true;
322+
}
323+
324+
if (pathInfo.isDirectory()) {
325+
return false;
326+
}
327+
328+
if (!pathInfo.isSymbolicLink()) {
329+
return true;
330+
}
331+
332+
try {
333+
// Git reports directory symlinks as untracked paths, but `git diff --no-index`
334+
// cannot synthesize a parseable file patch for them.
335+
return !fs.statSync(absolutePath).isDirectory();
336+
} catch {
337+
// Broken symlinks still diff as reviewable path entries, so keep them.
338+
return true;
339+
}
340+
}
341+
309342
/** Return the repo-root-relative untracked files for a working-tree review input. */
310343
export function listGitUntrackedFiles(
311344
input: GitCommandInput,
312-
{ cwd = process.cwd(), gitExecutable = "git" }: Omit<RunGitTextOptions, "input" | "args"> = {},
345+
{
346+
cwd = process.cwd(),
347+
repoRoot,
348+
gitExecutable = "git",
349+
}: Omit<RunGitTextOptions, "input" | "args"> & { repoRoot?: string } = {},
313350
) {
314351
if (!shouldIncludeUntrackedFiles(input)) {
315352
return [];
@@ -322,7 +359,15 @@ export function listGitUntrackedFiles(
322359
gitExecutable,
323360
});
324361

325-
return parseUntrackedFilePaths(statusText);
362+
const untrackedFiles = parseUntrackedFilePaths(statusText);
363+
if (untrackedFiles.length === 0) {
364+
return [];
365+
}
366+
367+
const normalizedRepoRoot = repoRoot ?? resolveGitRepoRoot(input, { cwd, gitExecutable });
368+
return untrackedFiles.filter((filePath) =>
369+
isReviewableUntrackedPath(normalizedRepoRoot, filePath),
370+
);
326371
}
327372

328373
/** Return the raw Git patch text for one untracked file using `git diff --no-index`. */

src/core/loaders.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterEach, describe, expect, test } from "bun:test";
2-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
2+
import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import { loadAppBootstrap } from "./loaders";
@@ -192,6 +192,30 @@ describe("loadAppBootstrap", () => {
192192
expect(bootstrap.changeset.files[1]?.patch).toContain("new file mode");
193193
});
194194

195+
test("skips untracked symlinks to directories while loading the rest of the review", async () => {
196+
const dir = createTempRepo("hunk-git-untracked-dir-symlink-");
197+
198+
writeFileSync(join(dir, "tracked.ts"), "export const tracked = 1;\n");
199+
git(dir, "add", "tracked.ts");
200+
git(dir, "commit", "-m", "initial");
201+
202+
writeFileSync(join(dir, "tracked.ts"), "export const tracked = 2;\n");
203+
writeFileSync(join(dir, "new-file.ts"), "export const added = true;\n");
204+
mkdirSync(join(dir, "targetdir"), { recursive: true });
205+
symlinkSync("targetdir", join(dir, "linkdir"));
206+
207+
const bootstrap = await loadFromRepo(dir, {
208+
kind: "git",
209+
staged: false,
210+
options: { mode: "auto" },
211+
});
212+
213+
expect(bootstrap.changeset.files.map((file) => file.path)).toEqual([
214+
"tracked.ts",
215+
"new-file.ts",
216+
]);
217+
});
218+
195219
test("can exclude untracked files from working tree reviews", async () => {
196220
const dir = createTempRepo("hunk-git-no-untracked-");
197221

src/core/loaders.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ async function loadGitChangeset(
392392
agentContext,
393393
);
394394
const trackedFiles = trackedChangeset.files;
395-
const untrackedFiles = listGitUntrackedFiles(input);
395+
const untrackedFiles = listGitUntrackedFiles(input, { cwd, repoRoot });
396396

397397
if (untrackedFiles.length === 0) {
398398
return trackedChangeset;

src/core/watch.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ function statSignature(path: string) {
3333
function gitWorkingTreeWatchSignature(input: Extract<CliInput, { kind: "git" }>) {
3434
const trackedPatch = runGitText({ input, args: buildGitDiffArgs(input) });
3535
const repoRoot = resolveGitRepoRoot(input);
36-
const untrackedSignatures = listGitUntrackedFiles(input).map(
36+
const untrackedSignatures = listGitUntrackedFiles(input, { repoRoot }).map(
3737
(filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`,
3838
);
3939

0 commit comments

Comments
 (0)