authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-06-20 10:59:10-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-06-20 10:59:10-07:00
logbf1a6501c7ada5822f0c68e8e5a0aa6bdbb9c1b7
tree71088f439a4d45cbc78ce61e9da590a51236e51c
parentf7867addbe18ebb55c916041dc168aa4b8f39335
signature Signed by PGP key B5690EEEBB952194

[codex] Preserve review path resolution failures (#3357)

Co-authored-by: codex <codex@users.noreply.github.com>

2 files changed, 42 insertions(+), 2 deletions(-)

apps/server/src/review/ReviewService.test.ts+24
...@@ -3,6 +3,7 @@ import { assert, describe, it } from "@effect/vitest";...@@ -3,6 +3,7 @@ import { assert, describe, it } from "@effect/vitest";
3import * as Effect from "effect/Effect";3import * as Effect from "effect/Effect";
4import * as FileSystem from "effect/FileSystem";4import * as FileSystem from "effect/FileSystem";
5import * as Layer from "effect/Layer";5import * as Layer from "effect/Layer";
6import * as PlatformError from "effect/PlatformError";
67
7import { ServerConfig } from "../config.ts";8import { ServerConfig } from "../config.ts";
8import * as GitVcsDriver from "../vcs/GitVcsDriver.ts";9import * as GitVcsDriver from "../vcs/GitVcsDriver.ts";
...@@ -73,4 +74,27 @@ describe("ReviewService", () => {...@@ -73,4 +74,27 @@ describe("ReviewService", () => {
73 assert.deepStrictEqual(detectCalls, [{ cwd: workspaceRoot }]);74 assert.deepStrictEqual(detectCalls, [{ cwd: workspaceRoot }]);
74 }).pipe(Effect.provide(NodeServices.layer)),75 }).pipe(Effect.provide(NodeServices.layer)),
75 );76 );
77
78 it.effect("preserves unexpected path-resolution failures", () =>
79 Effect.gen(function* () {
80 const fs = yield* FileSystem.FileSystem;
81 const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" });
82 const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" });
83 const invalidCwd = `${workspaceRoot}\0invalid`;
84 const detectCalls: Array<{ readonly cwd: string }> = [];
85
86 const error = yield* Effect.gen(function* () {
87 const review = yield* ReviewService.ReviewService;
88 return yield* review.getDiffPreview({ cwd: invalidCwd }).pipe(Effect.flip);
89 }).pipe(Effect.provide(makeLayer({ workspaceRoot, baseDir, detectCalls })));
90
91 assert.strictEqual(error._tag, "VcsRepositoryDetectionError");
92 if (error._tag !== "VcsRepositoryDetectionError") return;
93 assert.strictEqual(error.operation, "ReviewService.assertWorkspaceBoundCwd.canonicalizePath");
94 assert.strictEqual(error.cwd, invalidCwd);
95 assert.match(error.detail, /Failed to resolve a path/);
96 assert.instanceOf(error.cause, PlatformError.PlatformError);
97 assert.deepStrictEqual(detectCalls, []);
98 }).pipe(Effect.provide(NodeServices.layer)),
99 );
76});100});
apps/server/src/review/ReviewService.ts+18-2
...@@ -33,8 +33,24 @@ export const make = Effect.gen(function* () {...@@ -33,8 +33,24 @@ export const make = Effect.gen(function* () {
33 const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry;33 const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry;
34 const git = yield* GitVcsDriver.GitVcsDriver;34 const git = yield* GitVcsDriver.GitVcsDriver;
3535
36 const canonicalizePath = (value: string) =>36 const canonicalizePath = (value: string) => {
37 fileSystem.realPath(path.resolve(value)).pipe(Effect.orElseSucceed(() => path.resolve(value)));37 const resolvedPath = path.resolve(value);
38 return fileSystem.realPath(resolvedPath).pipe(
39 Effect.catchTags({
40 PlatformError: (cause) =>
41 cause.reason._tag === "NotFound"
42 ? Effect.succeed(resolvedPath)
43 : Effect.fail(
44 new VcsRepositoryDetectionError({
45 operation: "ReviewService.assertWorkspaceBoundCwd.canonicalizePath",
46 cwd: resolvedPath,
47 detail: "Failed to resolve a path while validating the review workspace.",
48 cause,
49 }),
50 ),
51 }),
52 );
53 };
3854
39 const isWithinRoot = (candidate: string, root: string) => {55 const isWithinRoot = (candidate: string, root: string) => {
40 const relative = path.relative(root, candidate);56 const relative = path.relative(root, candidate);