fix(review): use the selected thread's repository

apps/server/src/review/ReviewService.test.ts

@@ -3,131 +3,94 @@ import { assert, describe, it } from "@effect/vitest";
 import * as Effect from "effect/Effect";
 import * as FileSystem from "effect/FileSystem";
 import * as Layer from "effect/Layer";
-import * as PlatformError from "effect/PlatformError";
+import * as Path from "effect/Path";
 
 import { ServerConfig } from "../config.ts";
 import * as GitVcsDriver from "../vcs/GitVcsDriver.ts";
 import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";
+import * as VcsProcess from "../vcs/VcsProcess.ts";
 import * as ReviewService from "./ReviewService.ts";
 
-function makeLayer(input: {
-  readonly workspaceRoot: string;
-  readonly baseDir: string;
-  readonly detectCalls?: Array<{ readonly cwd: string }>;
-}) {
-  return ReviewService.layer.pipe(
-    Layer.provide(
-      Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({
-        get: () => Effect.die("unexpected VCS registry get"),
-        resolve: () => Effect.die("unexpected VCS registry resolve"),
-        detect: (request) =>
-          Effect.sync(() => {
-            input.detectCalls?.push({ cwd: request.cwd });
-            return null;
-          }),
-      }),
-    ),
-    Layer.provide(Layer.mock(GitVcsDriver.GitVcsDriver)({})),
-    Layer.provide(ServerConfig.layerTest(input.workspaceRoot, input.baseDir)),
-    Layer.provideMerge(NodeServices.layer),
-  );
-}
+const services = ReviewService.layer.pipe(
+  Layer.provide(VcsDriverRegistry.layer),
+  Layer.provide(VcsProcess.layer),
+  Layer.provideMerge(GitVcsDriver.layer),
+);
 
 describe("ReviewService", () => {
-  it.effect("rejects diff preview cwd outside the configured workspace roots", () =>
+  it.effect("switches previews and file contents between projects and external worktrees", () =>
     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 path = yield* Path.Path;
+      const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-switch-" });
+      const launchDir = path.join(root, "server");
+      const projectA = path.join(root, "project-a");
+      const projectB = path.join(root, "project-b");
+      const worktree = path.join(root, "external-worktree");
+      yield* fs.makeDirectory(launchDir);
+      yield* Effect.gen(function* () {
+        const git = yield* GitVcsDriver.GitVcsDriver;
         const review = yield* ReviewService.ReviewService;
-        return yield* review.getDiffPreview({ cwd: outsideRoot }).pipe(Effect.flip);
-      }).pipe(Effect.provide(makeLayer({ workspaceRoot, baseDir, detectCalls })));
-
-      assert.strictEqual(error._tag, "VcsRepositoryDetectionError");
-      assert.strictEqual(error.operation, "ReviewService.getDiffPreview");
-      assert.match(
-        "detail" in error ? error.detail : "",
-        /must stay within the configured workspace root/,
-      );
-      assert.deepStrictEqual(detectCalls, []);
-    }).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 runGit = (cwd: string, args: ReadonlyArray<string>) =>
+          git.execute({ operation: "ReviewService.test.git", cwd, args });
+        for (const cwd of [projectA, projectB]) {
+          yield* fs.makeDirectory(cwd);
+          yield* runGit(cwd, ["init"]);
+          yield* runGit(cwd, ["config", "user.email", "test@example.com"]);
+          yield* runGit(cwd, ["config", "user.name", "Test"]);
+          yield* fs.writeFileString(path.join(cwd, "file.txt"), "original\n");
+          yield* runGit(cwd, ["add", "."]);
+          yield* runGit(cwd, ["commit", "-m", "initial"]);
+        }
+        yield* runGit(projectA, ["worktree", "add", "-b", "external", worktree]);
+        const changes = new Map([
+          [projectA, "project A\n"],
+          [projectB, "project B\nsecond line\n"],
+          [worktree, "external worktree\nsecond line\nthird line\n"],
+        ]);
+        for (const [cwd, content] of changes) {
+          yield* fs.writeFileString(path.join(cwd, "file.txt"), content);
+        }
+        for (const cwd of [projectA, projectB, worktree, projectA]) {
+          const content = changes.get(cwd)!;
+          const preview = yield* review.getDiffPreview({ cwd });
+          assert.strictEqual(preview.cwd, cwd);
+          const dirty = preview.sources.find((source) => source.kind === "working-tree")!;
+          assert.include(dirty.diff, `+${content.split("\n")[0]}`);
+          const contents = yield* review.getDiffFileContents({
+            cwd,
+            sourceKind: "working-tree",
+            changeType: "change",
+            baseRef: "HEAD",
+            headRef: null,
+            oldPath: "file.txt",
+            newPath: "file.txt",
+          });
+          assert.strictEqual(contents.oldContents, "original\n");
+          assert.strictEqual(contents.newContents, content);
+        }
+        const empty = yield* review.getDiffPreview({ cwd: launchDir });
+        assert.strictEqual(empty.cwd, launchDir);
+        assert.deepStrictEqual(empty.sources, []);
 
-      const error = yield* Effect.gen(function* () {
-        const review = yield* ReviewService.ReviewService;
-        return yield* review
+        const escaped = yield* review
           .getDiffFileContents({
-            cwd: outsideRoot,
+            cwd: projectA,
             sourceKind: "working-tree",
-            changeType: "change",
+            changeType: "new",
             baseRef: "HEAD",
             headRef: null,
-            oldPath: "file.ts",
-            newPath: "file.ts",
+            oldPath: "../project-b/file.txt",
+            newPath: "../project-b/file.txt",
           })
           .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.strictEqual(escaped._tag, "GitCommandError");
+        if (escaped._tag === "GitCommandError") assert.include(escaped.detail, "outside");
+      }).pipe(
+        Effect.provide(
+          services.pipe(Layer.provide(ServerConfig.layerTest(launchDir, path.join(root, "state")))),
+        ),
       );
-      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;
-      const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" });
-      const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" });
-      const detectCalls: Array<{ readonly cwd: string }> = [];
-
-      const result = yield* Effect.gen(function* () {
-        const review = yield* ReviewService.ReviewService;
-        return yield* review.getDiffPreview({ cwd: workspaceRoot });
-      }).pipe(Effect.provide(makeLayer({ workspaceRoot, baseDir, detectCalls })));
-
-      assert.strictEqual(result.cwd, workspaceRoot);
-      assert.deepStrictEqual(result.sources, []);
-      assert.deepStrictEqual(detectCalls, [{ cwd: workspaceRoot }]);
-    }).pipe(Effect.provide(NodeServices.layer)),
-  );
-
-  it.effect("preserves unexpected path-resolution failures", () =>
-    Effect.gen(function* () {
-      const fs = yield* FileSystem.FileSystem;
-      const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" });
-      const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" });
-      const invalidCwd = `${workspaceRoot}\0invalid`;
-      const detectCalls: Array<{ readonly cwd: string }> = [];
-
-      const error = yield* Effect.gen(function* () {
-        const review = yield* ReviewService.ReviewService;
-        return yield* review.getDiffPreview({ cwd: invalidCwd }).pipe(Effect.flip);
-      }).pipe(Effect.provide(makeLayer({ workspaceRoot, baseDir, detectCalls })));
-
-      assert.strictEqual(error._tag, "VcsRepositoryDetectionError");
-      if (error._tag !== "VcsRepositoryDetectionError") return;
-      assert.strictEqual(error.operation, "ReviewService.assertWorkspaceBoundCwd.canonicalizePath");
-      assert.strictEqual(error.cwd, invalidCwd);
-      assert.match(error.detail, /Failed to resolve a path/);
-      assert.instanceOf(error.cause, PlatformError.PlatformError);
-      assert.deepStrictEqual(detectCalls, []);
     }).pipe(Effect.provide(NodeServices.layer)),
   );
 });

apps/server/src/review/ReviewService.ts

@@ -1,12 +1,9 @@
 import * as Context from "effect/Context";
 import * as DateTime from "effect/DateTime";
 import * as Effect from "effect/Effect";
-import * as FileSystem from "effect/FileSystem";
 import * as Layer from "effect/Layer";
-import * as Path from "effect/Path";
 
 import {
-  VcsRepositoryDetectionError,
   VcsUnsupportedOperationError,
   type ReviewDiffFileContentsInput,
   type ReviewDiffFileContentsResult,
@@ -15,7 +12,6 @@ import {
   type ReviewDiffPreviewResult,
 } from "@t3tools/contracts";
 
-import * as ServerConfig from "../config.ts";
 import * as GitVcsDriver from "../vcs/GitVcsDriver.ts";
 import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";
 
@@ -32,65 +28,14 @@ export class ReviewService extends Context.Service<
 >()("t3/review/ReviewService") {}
 
 export const make = Effect.gen(function* () {
-  const config = yield* ServerConfig.ServerConfig;
-  const fileSystem = yield* FileSystem.FileSystem;
-  const path = yield* Path.Path;
   const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry;
   const git = yield* GitVcsDriver.GitVcsDriver;
 
-  const canonicalizePath = (value: string) => {
-    const resolvedPath = path.resolve(value);
-    return fileSystem.realPath(resolvedPath).pipe(
-      Effect.catchTags({
-        PlatformError: (cause) =>
-          cause.reason._tag === "NotFound"
-            ? Effect.succeed(resolvedPath)
-            : Effect.fail(
-                new VcsRepositoryDetectionError({
-                  operation: "ReviewService.assertWorkspaceBoundCwd.canonicalizePath",
-                  cwd: resolvedPath,
-                  detail: "Failed to resolve a path while validating the review workspace.",
-                  cause,
-                }),
-              ),
-      }),
-    );
-  };
-
-  const isWithinRoot = (candidate: string, root: string) => {
-    const relative = path.relative(root, candidate);
-    return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
-  };
-
-  const assertWorkspaceBoundCwd = Effect.fn("ReviewService.assertWorkspaceBoundCwd")(function* (
-    operation: "ReviewService.getDiffPreview" | "ReviewService.getDiffFileContents",
-    cwd: string,
-  ) {
-    const [candidate, workspaceRoot, worktreesRoot] = yield* Effect.all([
-      canonicalizePath(cwd),
-      canonicalizePath(config.cwd),
-      canonicalizePath(config.worktreesDir),
-    ]);
-
-    if (isWithinRoot(candidate, workspaceRoot) || isWithinRoot(candidate, worktreesRoot)) {
-      return;
-    }
-
-    return yield* new VcsRepositoryDetectionError({
-      operation,
-      cwd,
-      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.",
-    });
-  });
-
+  // Review uses the requested repository, like the other VCS operations. The
+  // server's launch directory is only a default workspace, not a filesystem boundary.
   const getDiffPreview: ReviewService["Service"]["getDiffPreview"] = Effect.fn(
     "ReviewService.getDiffPreview",
   )(function* (input) {
-    yield* assertWorkspaceBoundCwd("ReviewService.getDiffPreview", input.cwd);
-
     const handle = yield* vcsRegistry.detect({ cwd: input.cwd, requestedKind: "auto" });
     if (!handle) {
       return {
@@ -118,8 +63,6 @@ export const make = Effect.gen(function* () {
   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({

apps/web/src/components/DiffPanel.tsx

@@ -256,7 +256,7 @@ export default function DiffPanel({
     },
     { enabled: isGitRepo && selectedTurn !== undefined },
   );
-  const primaryBranchDiffPreview = useEnvironmentQuery(
+  const branchDiffPreview = useEnvironmentQuery(
     selectedTurnId === null && activeThread && activeCwd
       ? reviewEnvironment.diffPreview({
           environmentId: activeThread.environmentId,
@@ -268,26 +268,6 @@ export default function DiffPanel({
         })
       : null,
   );
-  const shouldRetryBranchDiffAtEnvironmentCwd =
-    selectedTurnId === null &&
-    primaryBranchDiffPreview.error?.includes("configured workspace root") === true &&
-    serverConfig?.cwd !== undefined &&
-    serverConfig.cwd !== activeCwd;
-  const fallbackBranchDiffPreview = useEnvironmentQuery(
-    shouldRetryBranchDiffAtEnvironmentCwd && activeThread && serverConfig
-      ? reviewEnvironment.diffPreview({
-          environmentId: activeThread.environmentId,
-          input: {
-            cwd: serverConfig.cwd,
-            ...(selectedBaseRef ? { baseRef: selectedBaseRef } : {}),
-            ignoreWhitespace: diffIgnoreWhitespace,
-          },
-        })
-      : null,
-  );
-  const branchDiffPreview = shouldRetryBranchDiffAtEnvironmentCwd
-    ? fallbackBranchDiffPreview
-    : primaryBranchDiffPreview;
   const refreshBranchDiffPreview = branchDiffPreview.refresh;
   const canRefreshGitDiff =
     isGitRepo && selectedTurnId === null && activeThread != null && activeCwd != null;