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
1 change: 1 addition & 0 deletions apps/server/src/auth/RpcAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const RPC_REQUIRED_SCOPES = {
[WS_METHODS.vcsSwitchRef]: AuthOrchestrationOperateScope,
[WS_METHODS.vcsInit]: AuthOrchestrationOperateScope,
[WS_METHODS.reviewGetDiffPreview]: AuthReviewWriteScope,
[WS_METHODS.reviewGetDiffFileContents]: AuthReviewWriteScope,
[WS_METHODS.terminalOpen]: AuthTerminalOperateScope,
[WS_METHODS.terminalAttach]: AuthTerminalOperateScope,
[WS_METHODS.terminalWrite]: AuthTerminalOperateScope,
Expand Down
33 changes: 33 additions & 0 deletions apps/server/src/review/ReviewService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,39 @@ describe("ReviewService", () => {
}).pipe(Effect.provide(NodeServices.layer)),
);

it.effect("attributes file-content workspace violations to the file-content operation", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" });
const outsideRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-outside-" });
const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" });
const detectCalls: Array<{ readonly cwd: string }> = [];

const error = yield* Effect.gen(function* () {
const review = yield* ReviewService.ReviewService;
return yield* review
.getDiffFileContents({
cwd: outsideRoot,
sourceKind: "working-tree",
changeType: "change",
baseRef: "HEAD",
headRef: null,
oldPath: "file.ts",
newPath: "file.ts",
})
.pipe(Effect.flip);
}).pipe(Effect.provide(makeLayer({ workspaceRoot, baseDir, detectCalls })));

assert.strictEqual(error._tag, "VcsRepositoryDetectionError");
assert.strictEqual(error.operation, "ReviewService.getDiffFileContents");
assert.match(
"detail" in error ? error.detail : "",
/must stay within the configured workspace root/,
);
assert.deepStrictEqual(detectCalls, []);
}).pipe(Effect.provide(NodeServices.layer)),
);

it.effect("allows diff preview cwd inside the configured workspace root", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
Expand Down
33 changes: 30 additions & 3 deletions apps/server/src/review/ReviewService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import * as Path from "effect/Path";
import {
VcsRepositoryDetectionError,
VcsUnsupportedOperationError,
type ReviewDiffFileContentsInput,
type ReviewDiffFileContentsResult,
type ReviewDiffPreviewError,
type ReviewDiffPreviewInput,
type ReviewDiffPreviewResult,
Expand All @@ -23,6 +25,9 @@ export class ReviewService extends Context.Service<
readonly getDiffPreview: (
input: ReviewDiffPreviewInput,
) => Effect.Effect<ReviewDiffPreviewResult, ReviewDiffPreviewError>;
readonly getDiffFileContents: (
input: ReviewDiffFileContentsInput,
) => Effect.Effect<ReviewDiffFileContentsResult, ReviewDiffPreviewError>;
}
>()("t3/review/ReviewService") {}

Expand Down Expand Up @@ -58,6 +63,7 @@ export const make = Effect.gen(function* () {
};

const assertWorkspaceBoundCwd = Effect.fn("ReviewService.assertWorkspaceBoundCwd")(function* (
operation: "ReviewService.getDiffPreview" | "ReviewService.getDiffFileContents",
cwd: string,
) {
const [candidate, workspaceRoot, worktreesRoot] = yield* Effect.all([
Expand All @@ -71,16 +77,19 @@ export const make = Effect.gen(function* () {
}

return yield* new VcsRepositoryDetectionError({
operation: "ReviewService.getDiffPreview",
operation,
cwd,
detail: "Review diff preview cwd must stay within the configured workspace root.",
detail:
operation === "ReviewService.getDiffPreview"
? "Review diff preview cwd must stay within the configured workspace root."
: "Review diff file contents cwd must stay within the configured workspace root.",
});
});

const getDiffPreview: ReviewService["Service"]["getDiffPreview"] = Effect.fn(
"ReviewService.getDiffPreview",
)(function* (input) {
yield* assertWorkspaceBoundCwd(input.cwd);
yield* assertWorkspaceBoundCwd("ReviewService.getDiffPreview", input.cwd);

const handle = yield* vcsRegistry.detect({ cwd: input.cwd, requestedKind: "auto" });
if (!handle) {
Expand All @@ -106,8 +115,26 @@ export const make = Effect.gen(function* () {
return yield* getDriverDiffPreview(input);
});

const getDiffFileContents: ReviewService["Service"]["getDiffFileContents"] = Effect.fn(
"ReviewService.getDiffFileContents",
)(function* (input) {
yield* assertWorkspaceBoundCwd("ReviewService.getDiffFileContents", input.cwd);

const handle = yield* vcsRegistry.detect({ cwd: input.cwd, requestedKind: "auto" });
if (handle?.kind !== "git") {
return yield* new VcsUnsupportedOperationError({
operation: "ReviewService.getDiffFileContents",
kind: handle?.kind ?? "unknown",
detail: "Unchanged diff expansion currently requires a Git repository.",
});
}

return yield* git.getReviewDiffFileContents(input);
});

return ReviewService.of({
getDiffPreview,
getDiffFileContents,
});
});

Expand Down
21 changes: 21 additions & 0 deletions apps/server/src/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5229,6 +5229,11 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
},
],
}),
getDiffFileContents: () =>
Effect.succeed({
oldContents: "before\n",
newContents: "after\n",
}),
},
},
});
Expand Down Expand Up @@ -5343,6 +5348,22 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
),
);
assert.equal(diffPreview.sources[0]?.diff, "dirty-diff");

const diffFileContents = yield* Effect.scoped(
withWsRpcClient(wsUrl, (client) =>
client[WS_METHODS.reviewGetDiffFileContents]({
cwd: "/tmp/repo",
sourceKind: "working-tree",
changeType: "change",
baseRef: "HEAD",
headRef: null,
oldPath: "README.md",
newPath: "README.md",
}),
),
);
assert.equal(diffFileContents.oldContents, "before\n");
assert.equal(diffFileContents.newContents, "after\n");
}).pipe(Effect.provide(NodeHttpServer.layerTest)),
);

Expand Down
5 changes: 5 additions & 0 deletions apps/server/src/vcs/GitVcsDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
type VcsCreateWorktreeResult,
type ReviewDiffPreviewInput,
type ReviewDiffPreviewResult,
type ReviewDiffFileContentsInput,
type ReviewDiffFileContentsResult,
type VcsInitInput,
type VcsListRefsInput,
type VcsListRefsResult,
Expand Down Expand Up @@ -221,6 +223,9 @@ export class GitVcsDriver extends Context.Service<
readonly getReviewDiffPreview: (
input: ReviewDiffPreviewInput,
) => Effect.Effect<ReviewDiffPreviewResult, GitCommandError>;
readonly getReviewDiffFileContents: (
input: ReviewDiffFileContentsInput,
) => Effect.Effect<ReviewDiffFileContentsResult, GitCommandError>;
readonly readConfigValue: (
cwd: string,
key: string,
Expand Down
117 changes: 116 additions & 1 deletion apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import * as Stream from "effect/Stream";
import * as TestClock from "effect/testing/TestClock";
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";

import { GitCommandError } from "@t3tools/contracts";
import { GitCommandError, type ReviewDiffFileContentsInput } from "@t3tools/contracts";
import { ServerConfig } from "../config.ts";
import { makeGitVcsDriverCore, splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts";
import * as GitVcsDriver from "./GitVcsDriver.ts";
Expand Down Expand Up @@ -78,6 +78,20 @@ const writeTextFile = (
yield* fileSystem.writeFileString(filePath, contents);
});

const makeReviewDiffFileContentsInput = (
cwd: string,
overrides: Partial<Omit<ReviewDiffFileContentsInput, "cwd">> = {},
): ReviewDiffFileContentsInput => ({
cwd,
sourceKind: "working-tree",
changeType: "change",
baseRef: "HEAD",
headRef: null,
oldPath: "README.md",
newPath: "README.md",
...overrides,
});

const git = (
cwd: string,
args: ReadonlyArray<string>,
Expand Down Expand Up @@ -803,6 +817,107 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
);
}),
);

it.effect("loads full file contents for working-tree diff expansion", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
const pathService = yield* Path.Path;
yield* writeTextFile(cwd, "nested/.keep", "");
yield* writeTextFile(cwd, "README.md", "# changed\nunchanged context\n");

const contents = yield* driver.getReviewDiffFileContents(
makeReviewDiffFileContentsInput(pathService.join(cwd, "nested")),
);

assert.strictEqual(contents.oldContents, "# test\n");
assert.strictEqual(contents.newContents, "# changed\nunchanged context\n");
}),
);

it.effect("attributes working-tree filesystem failures to the failing operation", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;

const error = yield* driver
.getReviewDiffFileContents(
makeReviewDiffFileContentsInput(cwd, {
changeType: "new",
oldPath: "missing.ts",
newPath: "missing.ts",
}),
)
.pipe(Effect.flip);

assert.deepInclude(error, {
_tag: "GitCommandError",
operation: "GitVcsDriver.getReviewDiffFileContents.workingTree.fs.realPath",
command: "fs.realPath",
cwd,
detail: "Could not resolve diff file 'missing.ts'.",
});
}),
);

it.effect("loads new and deleted files without reading their missing side", () =>
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, "added.ts", "export const added = true;\n");
yield* fileSystem.remove(pathService.join(cwd, "README.md"));

const [added, deleted] = yield* Effect.all([
driver.getReviewDiffFileContents(
makeReviewDiffFileContentsInput(cwd, {
changeType: "new",
oldPath: "added.ts",
newPath: "added.ts",
}),
),
driver.getReviewDiffFileContents(
makeReviewDiffFileContentsInput(cwd, { changeType: "deleted" }),
),
]);

assert.deepStrictEqual(added, {
oldContents: "",
newContents: "export const added = true;\n",
});
assert.deepStrictEqual(deleted, {
oldContents: "# test\n",
newContents: "",
});
}),
);

it.effect("loads merge-base and head contents for branch diff expansion", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const { initialBranch } = yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* git(cwd, ["checkout", "-b", "feature/context"]);
yield* writeTextFile(cwd, "README.md", "# branch change\nunchanged context\n");
yield* git(cwd, ["add", "README.md"]);
yield* git(cwd, ["commit", "-m", "change readme"]);

const contents = yield* driver.getReviewDiffFileContents(
makeReviewDiffFileContentsInput(cwd, {
sourceKind: "branch-range",
baseRef: initialBranch,
headRef: "feature/context",
}),
);

assert.strictEqual(contents.oldContents, "# test\n");
assert.strictEqual(contents.newContents, "# branch change\nunchanged context\n");
}),
);
});

describe("repository status", () => {
Expand Down
Loading
Loading