Skip to content
Merged
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
116 changes: 115 additions & 1 deletion apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2170,6 +2170,14 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
.sources[0]!;
assert.equal(rootSource.baseRef, emptyTree);
assert.include(rootSource.diff, "+# test");
const rootMetadata = (yield* driver.getReviewDiffPreview({
cwd: upstream,
commitSha: root,
metadataOnly: true,
})).sources[0]!;
assert.deepStrictEqual(rootMetadata.files, rootSource.files);
assert.isTrue(rootMetadata.patchesDeferred);
assert.isEmpty(rootMetadata.diff);

yield* writeTextFile(upstream, "README.md", "# boundary\n");
yield* git(upstream, ["commit", "-am", "boundary"]);
Expand Down Expand Up @@ -2396,6 +2404,78 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
}),
);

it.effect(
"returns an incomplete selected tracked preview while untracked discovery is held",
() =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
yield* writeTextFile(cwd, "README.md", "changed\n");
yield* writeTextFile(cwd, "new.txt", "untracked\n");
const started = yield* Deferred.make<void>();
const release = yield* Deferred.make<void>();
let untrackedReads = 0;
const delegate = yield* ChildProcessSpawner.ChildProcessSpawner;
const spawner = ChildProcessSpawner.make((command) =>
Effect.gen(function* () {
if (ChildProcess.isStandardCommand(command) && command.args.includes("--others")) {
assert.equal(
++untrackedReads,
1,
"tracked preview must not enumerate untracked files",
);
yield* Deferred.succeed(started, undefined);
yield* Deferred.await(release);
}
return yield* delegate.spawn(command);
}),
);
const driver = yield* makeGitVcsDriverCore().pipe(
Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
Effect.provide(layerServerConfig),
);
const full = yield* driver
.getReviewDiffPreview({ cwd, sourceKind: "working-tree" })
.pipe(Effect.forkChild);
yield* Deferred.await(started);
yield* Effect.gen(function* () {
const preview = yield* driver.getReviewDiffPreview({
cwd,
sourceKind: "working-tree",
trackedOnly: true,
});
assert.isTrue(preview.incomplete);
assert.deepStrictEqual(
preview.sources.map((source) => source.kind),
["working-tree"],
);
assert.include(preview.sources[0]!.diff, "+changed");
assert.notInclude(preview.sources[0]!.diff, "untracked");
const metadata = yield* driver.getReviewDiffPreview({
cwd,
sourceKind: "working-tree",
trackedOnly: true,
metadataOnly: true,
});
assert.isTrue(metadata.incomplete);
assert.deepStrictEqual(metadata.sources[0]!.files, preview.sources[0]!.files);
assert.isTrue(metadata.sources[0]!.patchesDeferred);
assert.isEmpty(metadata.sources[0]!.diff);
}).pipe(Effect.ensuring(Deferred.succeed(release, undefined)));
const complete = yield* Fiber.join(full);
assert.isUndefined(complete.incomplete);
assert.include(complete.sources[0]!.diff, "+untracked");
yield* writeTextFile(cwd, "README.md", "edited again\n");
const refreshed = yield* driver.getReviewDiffPreview({
cwd,
baseRef: "missing-base",
sourceKind: "working-tree",
trackedOnly: true,
});
assert.include(refreshed.sources[0]!.diff, "+edited again");
}),
);

it.effect("propagates patch failures instead of reporting an empty complete diff", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
Expand All @@ -2416,6 +2496,16 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
if (Result.isFailure(result)) {
assert.equal(result.failure.operation, "GitVcsDriver.getReviewDiffPreview.patch");
}
const metadata = yield* driver.getReviewDiffPreview({
cwd,
sourceKind: "working-tree",
metadataOnly: true,
});
assert.isTrue(metadata.sources[0]!.patchesDeferred);
assert.isEmpty(metadata.sources[0]!.diff);
assert.deepStrictEqual(metadata.sources[0]!.files, [
{ path: "README.md", previousPath: null, additions: 1, deletions: 1 },
]);
}),
);

Expand Down Expand Up @@ -2597,9 +2687,17 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
);
yield* writeTextFile(cwd, "README.md", "# tracked change\n");

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

const source = preview.sources.find((source) => source.kind === "working-tree")!;
assert.isUndefined(source.patchesDeferred);
assert.isUndefined(source.files);
assert.isTrue(source.truncated);
assert.include(diff, "-# test");
assert.include(diff, "+# tracked change");
}),
Expand Down Expand Up @@ -2734,6 +2832,22 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
assert.isTrue(dirty.truncated);
assert.notInclude(branch.diff, "z-last.txt");
for (const source of [branch, dirty]) {
const selected = yield* driver.getReviewDiffPreview({
cwd,
baseRef: initialBranch,
sourceKind: source.kind === "working-tree" ? "working-tree" : "branch-range",
});
assert.deepStrictEqual(selected.sources, [source]);
const metadata = (yield* driver.getReviewDiffPreview({
cwd,
baseRef: initialBranch,
sourceKind: source.kind === "working-tree" ? "working-tree" : "branch-range",
metadataOnly: true,
})).sources[0]!;
assert.deepStrictEqual(metadata.files, source.files);
assert.isTrue(metadata.patchesDeferred);
assert.isFalse(metadata.truncated);
assert.isEmpty(metadata.diff);
for (const file of source.files ?? []) {
const individual = yield* driver.getReviewDiffPreview({
cwd,
Expand Down
50 changes: 35 additions & 15 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3409,10 +3409,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
const patchLimit = input.file
? REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES
: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES;
const sourceKind = input.file?.sourceKind ?? input.sourceKind;
const needsCurrentBranch =
input.commitSha === undefined &&
input.file?.sourceKind !== "commit" &&
input.file?.sourceKind !== "working-tree";
sourceKind !== "working-tree";
const repository = yield* (
needsCurrentBranch
? resolveRepositoryPathsUncached(input.cwd)
Expand Down Expand Up @@ -3463,12 +3464,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
{ maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES },
);
const files = parseReviewNumstat(stat.stdout);
const patch = yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.commitPatch",
cwd,
[...diffArgs, "--patch", commit.parent, commit.sha, "--", ...pathArgs],
{ maxOutputBytes: patchLimit, appendTruncationMarker: true },
);
const patch =
input.metadataOnly && !input.file
? { stdout: "", stdoutTruncated: false }
: yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.commitPatch",
cwd,
[...diffArgs, "--patch", commit.parent, commit.sha, "--", ...pathArgs],
{ maxOutputBytes: patchLimit, appendTruncationMarker: true },
);
const diffHash = yield* hashDiff(patch.stdout, files);
return {
cwd: input.cwd,
Expand All @@ -3485,14 +3489,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
files,
diffHash,
truncated: patch.stdoutTruncated,
...(input.metadataOnly && !input.file ? { patchesDeferred: true } : {}),
},
],
};
}

const dirtyRef = input.file?.sourceKind === "branch-range" ? null : "HEAD";
const dirtyRef = sourceKind === "branch-range" ? null : "HEAD";
const review =
input.file?.sourceKind === "working-tree"
sourceKind === "working-tree"
? { baseRef: input.baseRef ?? null, mergeBase: null }
: yield* resolveReviewMergeBase(cwd, repository.currentBranch, input.baseRef);

Expand Down Expand Up @@ -3534,10 +3539,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
const readTrackedDiff = Effect.fn("GitVcsDriver.getReviewDiffPreview.tracked")(function* (
ref: string | null,
env?: NodeJS.ProcessEnv,
metadataOnly = false,
) {
if (ref === null) return { stdout: "", stdoutTruncated: false, files: [] };
const stat = yield* readStats(ref, env);
if (stat.files.length === 0) return { stdout: "", stdoutTruncated: false, files: [] };
if (metadataOnly || stat.files.length === 0) {
return { stdout: "", stdoutTruncated: false, files: stat.files };
}
const patch = yield* executeGit(
"GitVcsDriver.getReviewDiffPreview.patch",
cwd,
Expand All @@ -3547,16 +3555,19 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
return { ...patch, files: stat.files };
});
const [dirtyTrackedResult, baseResult] = yield* Effect.gen(function* () {
const untracked = yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path);
const untracked = input.trackedOnly
? { env: undefined }
: yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path);
const metadataOnly = input.metadataOnly === true && !input.file && untracked !== null;
const [dirty, base] =
review.mergeBase === dirtyRef
? yield* readTrackedDiff(dirtyRef, untracked?.env).pipe(
? yield* readTrackedDiff(dirtyRef, untracked?.env, metadataOnly).pipe(
Effect.map((result) => [result, result] as const),
)
: yield* Effect.all(
[
readTrackedDiff(dirtyRef, untracked?.env),
readTrackedDiff(review.mergeBase, untracked?.env),
readTrackedDiff(dirtyRef, untracked?.env, metadataOnly),
readTrackedDiff(review.mergeBase, untracked?.env, metadataOnly),
],
{ concurrency: 2 },
);
Expand Down Expand Up @@ -3585,6 +3596,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
...(dirtyFiles === undefined ? {} : { files: dirtyFiles }),
diffHash: dirtyDiffHash,
truncated: dirtyTrackedResult.stdoutTruncated,
...(input.metadataOnly && !input.file && dirtyFiles !== undefined
? { patchesDeferred: true }
: {}),
},
{
id: "branch-range",
Expand All @@ -3596,13 +3610,19 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
...(baseFiles === undefined ? {} : { files: baseFiles }),
diffHash: baseDiffHash,
truncated: baseResult.stdoutTruncated,
...(input.metadataOnly && !input.file && baseFiles !== undefined
? { patchesDeferred: true }
: {}),
},
];

return {
cwd: input.cwd,
generatedAt: yield* DateTime.now,
sources,
sources: input.sourceKind
? sources.filter((source) => source.kind === input.sourceKind)
: sources,
...(input.trackedOnly ? { incomplete: true } : {}),
};
});

Expand Down
Loading
Loading