| author |  Jake Leventhal <jakeleventhal@me.com> 2026-09-11 10:34:08-04:00 |
| committer |  GitHub <noreply@github.com> 2026-09-11 11:34:08-03:00 |
| log | 26894dda7b6d78c5863ccec6581caf0ec7f4843b |
| tree | b2853ab2de795ddf4f6b443cd46d9c72f5fd7b0b |
| parent | fb3d165d33b155038de717851207d25fb52fe65e |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): detect file renames in review diffs (#8086)
2 files changed, 299 insertions(+), 32 deletions(-)
apps
server
apps/server/src/vcs/GitVcsDriverCore.test.ts+126
| ... | @@ -898,6 +898,132 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { | ... | @@ -898,6 +898,132 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { |
| 898 | }), | 898 | }), |
| 899 | ); | 899 | ); |
| 900 | 900 | ||
| 901 | it.effect("keeps untracked filenames with pathspec magic in the review", () => | ||
| 902 | Effect.gen(function* () { | ||
| 903 | const cwd = yield* makeTmpDir(); | ||
| 904 | yield* initRepoWithCommit(cwd); | ||
| 905 | const driver = yield* GitVcsDriver.GitVcsDriver; | ||
| 906 | yield* writeTextFile(cwd, ":(exclude)after.ts", "literal pathspec contents\n"); | ||
| 907 | yield* writeTextFile(cwd, "ordinary.ts", "ordinary contents\n"); | ||
| 908 | const indexBefore = yield* git(cwd, ["ls-files", "--stage"]); | ||
| 909 | |||
| 910 | const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); | ||
| 911 | const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; | ||
| 912 | |||
| 913 | assert.include(diff, "+literal pathspec contents"); | ||
| 914 | assert.include(diff, "+ordinary contents"); | ||
| 915 | assert.strictEqual(yield* git(cwd, ["ls-files", "--stage"]), indexBefore); | ||
| 916 | }), | ||
| 917 | ); | ||
| 918 | |||
| 919 | it.effect("detects an unstaged rename with edits without mutating a split index", () => | ||
| 920 | Effect.gen(function* () { | ||
| 921 | const cwd = yield* makeTmpDir(); | ||
| 922 | yield* initRepoWithCommit(cwd); | ||
| 923 | const driver = yield* GitVcsDriver.GitVcsDriver; | ||
| 924 | const fileSystem = yield* FileSystem.FileSystem; | ||
| 925 | const pathService = yield* Path.Path; | ||
| 926 | yield* writeTextFile(cwd, "before.ts", "one\ntwo\nthree\nfour\nfive\n"); | ||
| 927 | yield* git(cwd, ["add", "before.ts"]); | ||
| 928 | yield* git(cwd, ["commit", "-m", "add source file"]); | ||
| 929 | yield* git(cwd, ["config", "core.splitIndex", "true"]); | ||
| 930 | yield* git(cwd, ["config", "splitIndex.sharedIndexExpire", "now"]); | ||
| 931 | yield* git(cwd, ["update-index", "--split-index"]); | ||
| 932 | const indexPath = yield* git(cwd, ["rev-parse", "--git-path", "index"]); | ||
| 933 | const indexHashBefore = yield* git(cwd, ["hash-object", indexPath]); | ||
| 934 | const gitDirValue = yield* git(cwd, ["rev-parse", "--git-dir"]); | ||
| 935 | const gitDir = pathService.isAbsolute(gitDirValue) | ||
| 936 | ? gitDirValue | ||
| 937 | : pathService.resolve(cwd, gitDirValue); | ||
| 938 | const sharedIndexesBefore = (yield* fileSystem.readDirectory(gitDir)) | ||
| 939 | .filter((entry) => entry.startsWith("sharedindex.")) | ||
| 940 | .sort(); | ||
| 941 | yield* fileSystem.rename( | ||
| 942 | pathService.join(cwd, "before.ts"), | ||
| 943 | pathService.join(cwd, "after.ts"), | ||
| 944 | ); | ||
| 945 | yield* writeTextFile(cwd, "after.ts", "one\ntwo\nTHREE\nfour\nfive\n"); | ||
| 946 | |||
| 947 | const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); | ||
| 948 | const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; | ||
| 949 | const indexHashAfter = yield* git(cwd, ["hash-object", indexPath]); | ||
| 950 | const sharedIndexesAfter = (yield* fileSystem.readDirectory(gitDir)) | ||
| 951 | .filter((entry) => entry.startsWith("sharedindex.")) | ||
| 952 | .sort(); | ||
| 953 | |||
| 954 | assert.include(diff, "rename from before.ts"); | ||
| 955 | assert.include(diff, "rename to after.ts"); | ||
| 956 | assert.include(diff, "-three"); | ||
| 957 | assert.include(diff, "+THREE"); | ||
| 958 | assert.strictEqual(diff.match(/^diff --git /gm)?.length, 1); | ||
| 959 | assert.strictEqual(indexHashAfter, indexHashBefore); | ||
| 960 | assert.deepStrictEqual(sharedIndexesAfter, sharedIndexesBefore); | ||
| 961 | }), | ||
| 962 | ); | ||
| 963 | |||
| 964 | it.effect("keeps tracked changes visible when untracked discovery fails", () => | ||
| 965 | Effect.gen(function* () { | ||
| 966 | const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; | ||
| 967 | const failingLsFilesSpawner = ChildProcessSpawner.make((command) => { | ||
| 968 | if (!ChildProcess.isStandardCommand(command)) { | ||
| 969 | return Effect.die("expected a standard Git command"); | ||
| 970 | } | ||
| 971 | return command.args[0] === "ls-files" && command.args[1] === "--others" | ||
| 972 | ? Effect.succeed(makeNonRepositoryHandle()) | ||
| 973 | : delegate.spawn(command); | ||
| 974 | }); | ||
| 975 | const driver = yield* makeGitVcsDriverCore().pipe( | ||
| 976 | Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, failingLsFilesSpawner), | ||
| 977 | Effect.provide(ServerConfigLayer), | ||
| 978 | ); | ||
| 979 | const cwd = yield* makeTmpDir(); | ||
| 980 | yield* initRepoWithCommit(cwd).pipe( | ||
| 981 | Effect.provideService(GitVcsDriver.GitVcsDriver, driver), | ||
| 982 | ); | ||
| 983 | yield* writeTextFile(cwd, "README.md", "# tracked change\n"); | ||
| 984 | |||
| 985 | const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); | ||
| 986 | const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; | ||
| 987 | |||
| 988 | assert.include(diff, "-# test"); | ||
| 989 | assert.include(diff, "+# tracked change"); | ||
| 990 | }), | ||
| 991 | ); | ||
| 992 | |||
| 993 | it.effect("preserves a staged deletion when the removed path still exists", () => | ||
| 994 | Effect.gen(function* () { | ||
| 995 | const cwd = yield* makeTmpDir(); | ||
| 996 | yield* initRepoWithCommit(cwd); | ||
| 997 | const driver = yield* GitVcsDriver.GitVcsDriver; | ||
| 998 | yield* writeTextFile(cwd, "removed.txt", "remove me\n"); | ||
| 999 | yield* git(cwd, ["add", "removed.txt"]); | ||
| 1000 | yield* git(cwd, ["commit", "-m", "add removable file"]); | ||
| 1001 | yield* git(cwd, ["rm", "--cached", "removed.txt"]); | ||
| 1002 | |||
| 1003 | const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); | ||
| 1004 | const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; | ||
| 1005 | |||
| 1006 | assert.include(diff, "deleted file mode"); | ||
| 1007 | assert.include(diff, "-remove me"); | ||
| 1008 | assert.notInclude(diff, "new file mode"); | ||
| 1009 | }), | ||
| 1010 | ); | ||
| 1011 | |||
| 1012 | it.effect("keeps untracked files visible before the first commit", () => | ||
| 1013 | Effect.gen(function* () { | ||
| 1014 | const cwd = yield* makeTmpDir(); | ||
| 1015 | const driver = yield* GitVcsDriver.GitVcsDriver; | ||
| 1016 | yield* driver.initRepo({ cwd }); | ||
| 1017 | yield* writeTextFile(cwd, "untracked.txt", "visible before HEAD\n"); | ||
| 1018 | |||
| 1019 | const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); | ||
| 1020 | const source = preview.sources.find((candidate) => candidate.kind === "working-tree"); | ||
| 1021 | |||
| 1022 | assert.include(source?.diff, "visible before HEAD"); | ||
| 1023 | assert.equal(source?.truncated, false); | ||
| 1024 | }), | ||
| 1025 | ); | ||
| 1026 | |||
| 901 | it.effect("loads full file contents for working-tree diff expansion", () => | 1027 | it.effect("loads full file contents for working-tree diff expansion", () => |
| 902 | Effect.gen(function* () { | 1028 | Effect.gen(function* () { |
| 903 | const cwd = yield* makeTmpDir(); | 1029 | const cwd = yield* makeTmpDir(); |
apps/server/src/vcs/GitVcsDriverCore.ts+173-32
| ... | @@ -2268,6 +2268,174 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* | ... | @@ -2268,6 +2268,174 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* |
| 2268 | }; | 2268 | }; |
| 2269 | }); | 2269 | }); |
| 2270 | 2270 | ||
| 2271 | const readTrackedReviewDiff = Effect.fn("readTrackedReviewDiff")(function* ( | ||
| 2272 | cwd: string, | ||
| 2273 | ignoreWhitespace: boolean | undefined, | ||
| 2274 | ) { | ||
| 2275 | const result = yield* executeGit( | ||
| 2276 | "GitVcsDriver.readTrackedReviewDiff", | ||
| 2277 | cwd, | ||
| 2278 | [ | ||
| 2279 | "diff", | ||
| 2280 | "--patch", | ||
| 2281 | "--no-color", | ||
| 2282 | "--no-ext-diff", | ||
| 2283 | "--no-textconv", | ||
| 2284 | "--minimal", | ||
| 2285 | ...PATCH_RENDER_PREFIX_ARGS, | ||
| 2286 | "--find-renames", | ||
| 2287 | ...(ignoreWhitespace ? ["--ignore-all-space"] : []), | ||
| 2288 | "HEAD", | ||
| 2289 | "--", | ||
| 2290 | ], | ||
| 2291 | { | ||
| 2292 | maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES, | ||
| 2293 | appendTruncationMarker: true, | ||
| 2294 | }, | ||
| 2295 | ); | ||
| 2296 | return { diff: result.stdout, truncated: result.stdoutTruncated }; | ||
| 2297 | }); | ||
| 2298 | |||
| 2299 | const readUnifiedWorkingTreeReviewDiff = Effect.fn("readUnifiedWorkingTreeReviewDiff")(function* ( | ||
| 2300 | cwd: string, | ||
| 2301 | untrackedPaths: ReadonlyArray<string>, | ||
| 2302 | pathsTruncated: boolean, | ||
| 2303 | ignoreWhitespace: boolean | undefined, | ||
| 2304 | ) { | ||
| 2305 | const [stagedDeletionsStdout, indexValue] = yield* Effect.all( | ||
| 2306 | [ | ||
| 2307 | runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.stagedDeletions", cwd, [ | ||
| 2308 | "diff", | ||
| 2309 | "--cached", | ||
| 2310 | "--name-only", | ||
| 2311 | "--diff-filter=D", | ||
| 2312 | "-z", | ||
| 2313 | "HEAD", | ||
| 2314 | "--", | ||
| 2315 | ]), | ||
| 2316 | runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.indexPath", cwd, [ | ||
| 2317 | "rev-parse", | ||
| 2318 | "--git-path", | ||
| 2319 | "index", | ||
| 2320 | ]), | ||
| 2321 | ], | ||
| 2322 | { concurrency: 2 }, | ||
| 2323 | ); | ||
| 2324 | const stagedDeletions = new Set(stagedDeletionsStdout.split("\0").filter(Boolean)); | ||
| 2325 | const pathsToAdd = untrackedPaths.filter((relativePath) => !stagedDeletions.has(relativePath)); | ||
| 2326 | if (pathsToAdd.length === 0) { | ||
| 2327 | const tracked = yield* readTrackedReviewDiff(cwd, ignoreWhitespace); | ||
| 2328 | return { ...tracked, truncated: pathsTruncated || tracked.truncated }; | ||
| 2329 | } | ||
| 2330 | |||
| 2331 | const indexPath = path.isAbsolute(indexValue.trim()) | ||
| 2332 | ? indexValue.trim() | ||
| 2333 | : path.resolve(cwd, indexValue.trim()); | ||
| 2334 | const tempIndexPath = yield* fileSystem.makeTempFileScoped({ | ||
| 2335 | prefix: `t3code-review-index-${process.pid}-`, | ||
| 2336 | }); | ||
| 2337 | yield* fileSystem.copyFile(indexPath, tempIndexPath); | ||
| 2338 | const env = { GIT_INDEX_FILE: tempIndexPath } satisfies NodeJS.ProcessEnv; | ||
| 2339 | const tempIndexConfig = [ | ||
| 2340 | "-c", | ||
| 2341 | "core.splitIndex=false", | ||
| 2342 | "-c", | ||
| 2343 | "splitIndex.sharedIndexExpire=never", | ||
| 2344 | ]; | ||
| 2345 | yield* executeGit( | ||
| 2346 | "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.expandSplitIndex", | ||
| 2347 | cwd, | ||
| 2348 | [...tempIndexConfig, "update-index", "--no-split-index"], | ||
| 2349 | { env }, | ||
| 2350 | ); | ||
| 2351 | yield* executeGit( | ||
| 2352 | "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.addUntracked", | ||
| 2353 | cwd, | ||
| 2354 | [ | ||
| 2355 | ...tempIndexConfig, | ||
| 2356 | "--literal-pathspecs", | ||
| 2357 | "add", | ||
| 2358 | "--intent-to-add", | ||
| 2359 | "--pathspec-from-file=-", | ||
| 2360 | "--pathspec-file-nul", | ||
| 2361 | ], | ||
| 2362 | { env, stdin: `${pathsToAdd.join("\0")}\0` }, | ||
| 2363 | ); | ||
| 2364 | const result = yield* executeGit( | ||
| 2365 | "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.diff", | ||
| 2366 | cwd, | ||
| 2367 | [ | ||
| 2368 | ...tempIndexConfig, | ||
| 2369 | "diff", | ||
| 2370 | "--patch", | ||
| 2371 | "--no-color", | ||
| 2372 | "--no-ext-diff", | ||
| 2373 | "--no-textconv", | ||
| 2374 | "--minimal", | ||
| 2375 | ...PATCH_RENDER_PREFIX_ARGS, | ||
| 2376 | "--find-renames", | ||
| 2377 | ...(ignoreWhitespace ? ["--ignore-all-space"] : []), | ||
| 2378 | "HEAD", | ||
| 2379 | "--", | ||
| 2380 | ], | ||
| 2381 | { | ||
| 2382 | env, | ||
| 2383 | maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES, | ||
| 2384 | appendTruncationMarker: true, | ||
| 2385 | }, | ||
| 2386 | ); | ||
| 2387 | return { diff: result.stdout, truncated: pathsTruncated || result.stdoutTruncated }; | ||
| 2388 | }); | ||
| 2389 | |||
| 2390 | const readWorkingTreeReviewDiff = Effect.fn("readWorkingTreeReviewDiff")(function* ( | ||
| 2391 | cwd: string, | ||
| 2392 | ignoreWhitespace: boolean | undefined, | ||
| 2393 | ) { | ||
| 2394 | const untrackedResult = yield* executeGit( | ||
| 2395 | "GitVcsDriver.readWorkingTreeReviewDiff.listUntracked", | ||
| 2396 | cwd, | ||
| 2397 | ["ls-files", "--others", "--exclude-standard", "-z"], | ||
| 2398 | { | ||
| 2399 | maxOutputBytes: WORKSPACE_FILES_MAX_OUTPUT_BYTES, | ||
| 2400 | appendTruncationMarker: true, | ||
| 2401 | }, | ||
| 2402 | ).pipe(Effect.option); | ||
| 2403 | if (untrackedResult._tag === "None") { | ||
| 2404 | return yield* readTrackedReviewDiff(cwd, ignoreWhitespace); | ||
| 2405 | } | ||
| 2406 | const untrackedPaths = splitNullSeparatedGitStdoutPaths(untrackedResult.value); | ||
| 2407 | if (untrackedPaths.length === 0) { | ||
| 2408 | const tracked = yield* readTrackedReviewDiff(cwd, ignoreWhitespace); | ||
| 2409 | return { ...tracked, truncated: untrackedResult.value.stdoutTruncated || tracked.truncated }; | ||
| 2410 | } | ||
| 2411 | |||
| 2412 | return yield* readUnifiedWorkingTreeReviewDiff( | ||
| 2413 | cwd, | ||
| 2414 | untrackedPaths, | ||
| 2415 | untrackedResult.value.stdoutTruncated, | ||
| 2416 | ignoreWhitespace, | ||
| 2417 | ).pipe( | ||
| 2418 | Effect.scoped, | ||
| 2419 | Effect.catch(() => | ||
| 2420 | Effect.all([ | ||
| 2421 | readTrackedReviewDiff(cwd, ignoreWhitespace).pipe( | ||
| 2422 | Effect.orElseSucceed(() => ({ diff: "", truncated: false })), | ||
| 2423 | ), | ||
| 2424 | readUntrackedReviewDiffs(cwd).pipe( | ||
| 2425 | Effect.orElseSucceed(() => ({ diff: "", truncated: false })), | ||
| 2426 | ), | ||
| 2427 | ]).pipe( | ||
| 2428 | Effect.map(([tracked, untracked]) => ({ | ||
| 2429 | diff: [tracked.diff.trimEnd(), untracked.diff.trimEnd()] | ||
| 2430 | .filter((diff) => diff.length > 0) | ||
| 2431 | .join("\n"), | ||
| 2432 | truncated: tracked.truncated || untracked.truncated, | ||
| 2433 | })), | ||
| 2434 | ), | ||
| 2435 | ), | ||
| 2436 | ); | ||
| 2437 | }); | ||
| 2438 | |||
| 2271 | const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* ( | 2439 | const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* ( |
| 2272 | input: ReviewDiffPreviewInput, | 2440 | input: ReviewDiffPreviewInput, |
| 2273 | ) { | 2441 | ) { |
| ... | @@ -2289,40 +2457,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* | ... | @@ -2289,40 +2457,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* |
| 2289 | ) | 2457 | ) |
| 2290 | : null); | 2458 | : null); |
| 2291 | 2459 | ||
| 2292 | const dirtyTrackedResult = yield* executeGit( | 2460 | const dirtyResult = yield* readWorkingTreeReviewDiff(input.cwd, input.ignoreWhitespace).pipe( |
| 2293 | "GitVcsDriver.getReviewDiffPreview.dirtyTracked", | ||
| 2294 | input.cwd, | ||
| 2295 | [ | ||
| 2296 | "diff", | ||
| 2297 | "--patch", | ||
| 2298 | "--no-color", | ||
| 2299 | "--no-ext-diff", | ||
| 2300 | "--no-textconv", | ||
| 2301 | "--minimal", | ||
| 2302 | ...PATCH_RENDER_PREFIX_ARGS, | ||
| 2303 | ...(input.ignoreWhitespace ? ["--ignore-all-space"] : []), | ||
| 2304 | "HEAD", | ||
| 2305 | "--", | ||
| 2306 | ], | ||
| 2307 | { | ||
| 2308 | maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES, | ||
| 2309 | appendTruncationMarker: true, | ||
| 2310 | }, | ||
| 2311 | ).pipe( | ||
| 2312 | Effect.orElseSucceed(() => ({ | 2461 | Effect.orElseSucceed(() => ({ |
| 2313 | exitCode: 0, | 2462 | diff: "", |
| 2314 | stdout: "", | 2463 | truncated: false, |
| 2315 | stderr: "", | ||
| 2316 | stdoutTruncated: false, | ||
| 2317 | stderrTruncated: false, | ||
| 2318 | })), | 2464 | })), |
| 2319 | ); | 2465 | ); |
| 2320 | const dirtyUntracked = yield* readUntrackedReviewDiffs(input.cwd).pipe( | 2466 | const dirtyDiff = dirtyResult.diff; |
| 2321 | Effect.orElseSucceed(() => ({ diff: "", truncated: false })), | ||
| 2322 | ); | ||
| 2323 | const dirtyDiff = [dirtyTrackedResult.stdout.trimEnd(), dirtyUntracked.diff.trimEnd()] | ||
| 2324 | .filter((diff) => diff.length > 0) | ||
| 2325 | .join("\n"); | ||
| 2326 | 2467 | ||
| 2327 | const baseResult = | 2468 | const baseResult = |
| 2328 | baseRef && branch | 2469 | baseRef && branch |
| ... | @@ -2383,7 +2524,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* | ... | @@ -2383,7 +2524,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* |
| 2383 | headRef: null, | 2524 | headRef: null, |
| 2384 | diff: dirtyDiff, | 2525 | diff: dirtyDiff, |
| 2385 | diffHash: dirtyDiffHash, | 2526 | diffHash: dirtyDiffHash, |
| 2386 | truncated: dirtyTrackedResult.stdoutTruncated || dirtyUntracked.truncated, | 2527 | truncated: dirtyResult.truncated, |
| 2387 | }, | 2528 | }, |
| 2388 | { | 2529 | { |
| 2389 | id: "branch-range", | 2530 | id: "branch-range", |