diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index fb741b28ee7c..4f1f0204a4ed 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -898,6 +898,132 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }), ); + it.effect("keeps untracked filenames with pathspec magic in the review", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* writeTextFile(cwd, ":(exclude)after.ts", "literal pathspec contents\n"); + yield* writeTextFile(cwd, "ordinary.ts", "ordinary contents\n"); + const indexBefore = yield* git(cwd, ["ls-files", "--stage"]); + + const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); + const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; + + assert.include(diff, "+literal pathspec contents"); + assert.include(diff, "+ordinary contents"); + assert.strictEqual(yield* git(cwd, ["ls-files", "--stage"]), indexBefore); + }), + ); + + it.effect("detects an unstaged rename with edits without mutating a split index", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; + const pathService = yield* Path.Path; + yield* writeTextFile(cwd, "before.ts", "one\ntwo\nthree\nfour\nfive\n"); + yield* git(cwd, ["add", "before.ts"]); + yield* git(cwd, ["commit", "-m", "add source file"]); + yield* git(cwd, ["config", "core.splitIndex", "true"]); + yield* git(cwd, ["config", "splitIndex.sharedIndexExpire", "now"]); + yield* git(cwd, ["update-index", "--split-index"]); + const indexPath = yield* git(cwd, ["rev-parse", "--git-path", "index"]); + const indexHashBefore = yield* git(cwd, ["hash-object", indexPath]); + const gitDirValue = yield* git(cwd, ["rev-parse", "--git-dir"]); + const gitDir = pathService.isAbsolute(gitDirValue) + ? gitDirValue + : pathService.resolve(cwd, gitDirValue); + const sharedIndexesBefore = (yield* fileSystem.readDirectory(gitDir)) + .filter((entry) => entry.startsWith("sharedindex.")) + .sort(); + yield* fileSystem.rename( + pathService.join(cwd, "before.ts"), + pathService.join(cwd, "after.ts"), + ); + yield* writeTextFile(cwd, "after.ts", "one\ntwo\nTHREE\nfour\nfive\n"); + + const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); + const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; + const indexHashAfter = yield* git(cwd, ["hash-object", indexPath]); + const sharedIndexesAfter = (yield* fileSystem.readDirectory(gitDir)) + .filter((entry) => entry.startsWith("sharedindex.")) + .sort(); + + assert.include(diff, "rename from before.ts"); + assert.include(diff, "rename to after.ts"); + assert.include(diff, "-three"); + assert.include(diff, "+THREE"); + assert.strictEqual(diff.match(/^diff --git /gm)?.length, 1); + assert.strictEqual(indexHashAfter, indexHashBefore); + assert.deepStrictEqual(sharedIndexesAfter, sharedIndexesBefore); + }), + ); + + it.effect("keeps tracked changes visible when untracked discovery fails", () => + Effect.gen(function* () { + const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; + const failingLsFilesSpawner = ChildProcessSpawner.make((command) => { + if (!ChildProcess.isStandardCommand(command)) { + return Effect.die("expected a standard Git command"); + } + return command.args[0] === "ls-files" && command.args[1] === "--others" + ? Effect.succeed(makeNonRepositoryHandle()) + : delegate.spawn(command); + }); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, failingLsFilesSpawner), + Effect.provide(ServerConfigLayer), + ); + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd).pipe( + Effect.provideService(GitVcsDriver.GitVcsDriver, driver), + ); + yield* writeTextFile(cwd, "README.md", "# tracked change\n"); + + const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); + const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; + + assert.include(diff, "-# test"); + assert.include(diff, "+# tracked change"); + }), + ); + + it.effect("preserves a staged deletion when the removed path still exists", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* writeTextFile(cwd, "removed.txt", "remove me\n"); + yield* git(cwd, ["add", "removed.txt"]); + yield* git(cwd, ["commit", "-m", "add removable file"]); + yield* git(cwd, ["rm", "--cached", "removed.txt"]); + + const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); + const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; + + assert.include(diff, "deleted file mode"); + assert.include(diff, "-remove me"); + assert.notInclude(diff, "new file mode"); + }), + ); + + it.effect("keeps untracked files visible before the first commit", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* driver.initRepo({ cwd }); + yield* writeTextFile(cwd, "untracked.txt", "visible before HEAD\n"); + + const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); + const source = preview.sources.find((candidate) => candidate.kind === "working-tree"); + + assert.include(source?.diff, "visible before HEAD"); + assert.equal(source?.truncated, false); + }), + ); + it.effect("loads full file contents for working-tree diff expansion", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 3c7e018ddea7..d371e63617f6 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -2268,6 +2268,174 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }; }); + const readTrackedReviewDiff = Effect.fn("readTrackedReviewDiff")(function* ( + cwd: string, + ignoreWhitespace: boolean | undefined, + ) { + const result = yield* executeGit( + "GitVcsDriver.readTrackedReviewDiff", + cwd, + [ + "diff", + "--patch", + "--no-color", + "--no-ext-diff", + "--no-textconv", + "--minimal", + ...PATCH_RENDER_PREFIX_ARGS, + "--find-renames", + ...(ignoreWhitespace ? ["--ignore-all-space"] : []), + "HEAD", + "--", + ], + { + maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES, + appendTruncationMarker: true, + }, + ); + return { diff: result.stdout, truncated: result.stdoutTruncated }; + }); + + const readUnifiedWorkingTreeReviewDiff = Effect.fn("readUnifiedWorkingTreeReviewDiff")(function* ( + cwd: string, + untrackedPaths: ReadonlyArray, + pathsTruncated: boolean, + ignoreWhitespace: boolean | undefined, + ) { + const [stagedDeletionsStdout, indexValue] = yield* Effect.all( + [ + runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.stagedDeletions", cwd, [ + "diff", + "--cached", + "--name-only", + "--diff-filter=D", + "-z", + "HEAD", + "--", + ]), + runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.indexPath", cwd, [ + "rev-parse", + "--git-path", + "index", + ]), + ], + { concurrency: 2 }, + ); + const stagedDeletions = new Set(stagedDeletionsStdout.split("\0").filter(Boolean)); + const pathsToAdd = untrackedPaths.filter((relativePath) => !stagedDeletions.has(relativePath)); + if (pathsToAdd.length === 0) { + const tracked = yield* readTrackedReviewDiff(cwd, ignoreWhitespace); + return { ...tracked, truncated: pathsTruncated || tracked.truncated }; + } + + const indexPath = path.isAbsolute(indexValue.trim()) + ? indexValue.trim() + : path.resolve(cwd, indexValue.trim()); + const tempIndexPath = yield* fileSystem.makeTempFileScoped({ + prefix: `t3code-review-index-${process.pid}-`, + }); + yield* fileSystem.copyFile(indexPath, tempIndexPath); + const env = { GIT_INDEX_FILE: tempIndexPath } satisfies NodeJS.ProcessEnv; + const tempIndexConfig = [ + "-c", + "core.splitIndex=false", + "-c", + "splitIndex.sharedIndexExpire=never", + ]; + yield* executeGit( + "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.expandSplitIndex", + cwd, + [...tempIndexConfig, "update-index", "--no-split-index"], + { env }, + ); + yield* executeGit( + "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.addUntracked", + cwd, + [ + ...tempIndexConfig, + "--literal-pathspecs", + "add", + "--intent-to-add", + "--pathspec-from-file=-", + "--pathspec-file-nul", + ], + { env, stdin: `${pathsToAdd.join("\0")}\0` }, + ); + const result = yield* executeGit( + "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.diff", + cwd, + [ + ...tempIndexConfig, + "diff", + "--patch", + "--no-color", + "--no-ext-diff", + "--no-textconv", + "--minimal", + ...PATCH_RENDER_PREFIX_ARGS, + "--find-renames", + ...(ignoreWhitespace ? ["--ignore-all-space"] : []), + "HEAD", + "--", + ], + { + env, + maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES, + appendTruncationMarker: true, + }, + ); + return { diff: result.stdout, truncated: pathsTruncated || result.stdoutTruncated }; + }); + + const readWorkingTreeReviewDiff = Effect.fn("readWorkingTreeReviewDiff")(function* ( + cwd: string, + ignoreWhitespace: boolean | undefined, + ) { + const untrackedResult = yield* executeGit( + "GitVcsDriver.readWorkingTreeReviewDiff.listUntracked", + cwd, + ["ls-files", "--others", "--exclude-standard", "-z"], + { + maxOutputBytes: WORKSPACE_FILES_MAX_OUTPUT_BYTES, + appendTruncationMarker: true, + }, + ).pipe(Effect.option); + if (untrackedResult._tag === "None") { + return yield* readTrackedReviewDiff(cwd, ignoreWhitespace); + } + const untrackedPaths = splitNullSeparatedGitStdoutPaths(untrackedResult.value); + if (untrackedPaths.length === 0) { + const tracked = yield* readTrackedReviewDiff(cwd, ignoreWhitespace); + return { ...tracked, truncated: untrackedResult.value.stdoutTruncated || tracked.truncated }; + } + + return yield* readUnifiedWorkingTreeReviewDiff( + cwd, + untrackedPaths, + untrackedResult.value.stdoutTruncated, + ignoreWhitespace, + ).pipe( + Effect.scoped, + Effect.catch(() => + Effect.all([ + readTrackedReviewDiff(cwd, ignoreWhitespace).pipe( + Effect.orElseSucceed(() => ({ diff: "", truncated: false })), + ), + readUntrackedReviewDiffs(cwd).pipe( + Effect.orElseSucceed(() => ({ diff: "", truncated: false })), + ), + ]).pipe( + Effect.map(([tracked, untracked]) => ({ + diff: [tracked.diff.trimEnd(), untracked.diff.trimEnd()] + .filter((diff) => diff.length > 0) + .join("\n"), + truncated: tracked.truncated || untracked.truncated, + })), + ), + ), + ); + }); + const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* ( input: ReviewDiffPreviewInput, ) { @@ -2289,40 +2457,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ) : null); - const dirtyTrackedResult = yield* executeGit( - "GitVcsDriver.getReviewDiffPreview.dirtyTracked", - input.cwd, - [ - "diff", - "--patch", - "--no-color", - "--no-ext-diff", - "--no-textconv", - "--minimal", - ...PATCH_RENDER_PREFIX_ARGS, - ...(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: "", - stderr: "", - stdoutTruncated: false, - stderrTruncated: false, + diff: "", + truncated: 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.diff; const baseResult = baseRef && branch @@ -2383,7 +2524,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* headRef: null, diff: dirtyDiff, diffHash: dirtyDiffHash, - truncated: dirtyTrackedResult.stdoutTruncated || dirtyUntracked.truncated, + truncated: dirtyResult.truncated, }, { id: "branch-range",