From f77845272c15564c3a5c1c7e63054f80d3870c02 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 01:36:41 -0700 Subject: [PATCH 1/9] Make worktree placement configurable --- apps/server/src/git/GitManager.ts | 9 +++ apps/server/src/git/GitWorkflowService.ts | 2 +- apps/server/src/server.test.ts | 7 +++ apps/server/src/serverSettings.test.ts | 4 ++ apps/server/src/vcs/GitVcsDriver.ts | 2 +- apps/server/src/vcs/GitVcsDriverCore.test.ts | 56 +++++++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 12 +++- apps/server/src/ws.ts | 17 +++++- .../components/settings/SettingsPanels.tsx | 37 ++++++++++++ packages/contracts/src/settings.test.ts | 10 ++++ packages/contracts/src/settings.ts | 5 ++ 11 files changed, 157 insertions(+), 4 deletions(-) diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 95e5affc210..83e46660603 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -13,6 +13,7 @@ import * as Order from "effect/Order"; import * as Path from "effect/Path"; import * as Ref from "effect/Ref"; import { + DEFAULT_WORKTREE_PATH_TEMPLATE, GitActionProgressEvent, GitActionProgressPhase, GitCommandError, @@ -1877,6 +1878,14 @@ export const make = Effect.gen(function* () { cwd: input.cwd, refName: localPullRequestBranch, path: null, + pathTemplate: yield* serverSettingsService.getSettings.pipe( + Effect.map((settings) => settings.worktreePathTemplate), + Effect.catch((cause) => + Effect.logWarning("Failed to read worktree path template; using the default", { + cause, + }).pipe(Effect.as(DEFAULT_WORKTREE_PATH_TEMPLATE)), + ), + ), }); yield* ensureExistingWorktreeUpstream(worktree.worktree.path); yield* maybeRunSetupScript(worktree.worktree.path); diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index 100b9beadba..499a1451197 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -63,7 +63,7 @@ export class GitWorkflowService extends Context.Service< input: VcsListRefsInput, ) => Effect.Effect; readonly createWorktree: ( - input: VcsCreateWorktreeInput, + input: VcsCreateWorktreeInput & { readonly pathTemplate?: string }, ) => Effect.Effect; readonly fetchRemote: (input: { readonly cwd: string; diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index a3b2e3715fd..1dfea606b69 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -6858,6 +6858,12 @@ it.layer(NodeServices.layer)("server router seam", (it) => { yield* buildAppUnderTest({ layers: { + serverSettings: { + getSettings: Effect.succeed({ + ...DEFAULT_SERVER_SETTINGS, + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", + }), + }, gitVcsDriver: { fetchRemote, resolveRemoteTrackingCommit, @@ -6938,6 +6944,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { newRefName: "t3code/bootstrap-refName", baseRefName: "main", path: null, + pathTemplate: "{repoRoot}/.worktrees/{branch}", }); assert.deepEqual(fetchRemote.mock.calls[0]?.[0], { cwd: "/tmp/project", diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 50ca810a95a..afc1e8ba4ac 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -514,6 +514,7 @@ it.layer(NodeServices.layer)("server settings", (it) => { const next = yield* serverSettings.updateSettings({ addProjectBaseDirectory: " ~/Development ", + worktreePathTemplate: " {repoRoot}/.worktrees/{branch} ", observability: { otlpTracesUrl: " http://localhost:4318/v1/traces ", otlpMetricsUrl: " http://localhost:4318/v1/metrics ", @@ -521,6 +522,7 @@ it.layer(NodeServices.layer)("server settings", (it) => { }); assert.equal(next.addProjectBaseDirectory, "~/Development"); + assert.equal(next.worktreePathTemplate, "{repoRoot}/.worktrees/{branch}"); assert.deepEqual(next.observability, { otlpTracesUrl: "http://localhost:4318/v1/traces", otlpMetricsUrl: "http://localhost:4318/v1/metrics", @@ -555,6 +557,7 @@ it.layer(NodeServices.layer)("server settings", (it) => { const fileSystem = yield* FileSystem.FileSystem; const next = yield* serverSettings.updateSettings({ addProjectBaseDirectory: "~/Development", + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", observability: { otlpTracesUrl: "http://localhost:4318/v1/traces", otlpMetricsUrl: "http://localhost:4318/v1/metrics", @@ -577,6 +580,7 @@ it.layer(NodeServices.layer)("server settings", (it) => { // @effect-diagnostics-next-line preferSchemaOverJson:off assert.deepEqual(JSON.parse(raw), { addProjectBaseDirectory: "~/Development", + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", observability: { otlpTracesUrl: "http://localhost:4318/v1/traces", otlpMetricsUrl: "http://localhost:4318/v1/metrics", diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 55aa8f38835..216a6cc7761 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -230,7 +230,7 @@ export class GitVcsDriver extends Context.Service< ) => Effect.Effect; readonly pullCurrentBranch: (cwd: string) => Effect.Effect; readonly createWorktree: ( - input: VcsCreateWorktreeInput, + input: VcsCreateWorktreeInput & { readonly pathTemplate?: string }, ) => Effect.Effect; readonly fetchPullRequestBranch: ( input: GitFetchPullRequestBranchInput, diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 7b5956de07f..c3d7e40340e 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -134,6 +134,29 @@ it.effect("uses stable diagnostics for every parsed non-repository command", () }).pipe(Effect.provide(layer)); }); +it.effect("uses the configured repository-local worktree path template", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir("template-repo-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const driver = yield* GitVcsDriver.GitVcsDriver; + + const created = yield* driver.createWorktree({ + cwd, + path: null, + refName: initialBranch, + newRefName: "feature/local-worktree", + pathTemplate: "{repoRoot}/.worktrees/{branch}", + }); + + const expectedPath = pathService.join(cwd, ".worktrees", "feature-local-worktree"); + assert.equal(created.worktree.path, expectedPath); + assert.equal(yield* git(expectedPath, ["branch", "--show-current"]), "feature/local-worktree"); + + yield* driver.removeWorktree({ cwd, path: expectedPath }); + }).pipe(Effect.provide(TestLayer)), +); + it.layer(TestLayer)("GitVcsDriver core integration", (it) => { describe("process environment", () => { it.effect("preserves the caller locale for general Git subprocesses", () => @@ -661,6 +684,39 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }); describe("worktree operations", () => { + it.effect("keeps the centralized worktree path as the default", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir("default-template-repo-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const driver = yield* GitVcsDriver.GitVcsDriver; + + const created = yield* driver.createWorktree({ + cwd, + path: null, + refName: initialBranch, + newRefName: "feature/default-worktree", + }); + + const expectedPath = created.worktree.path; + assert.equal(pathService.basename(expectedPath), "feature-default-worktree"); + assert.equal( + pathService.basename(pathService.dirname(expectedPath)), + pathService.basename(cwd), + ); + assert.equal( + pathService.basename(pathService.dirname(pathService.dirname(expectedPath))), + "worktrees", + ); + assert.equal( + yield* git(expectedPath, ["branch", "--show-current"]), + "feature/default-worktree", + ); + + yield* driver.removeWorktree({ cwd, path: expectedPath }); + }), + ); + it.effect("creates and removes a worktree for a new refName", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 33eb6d40d76..da2b7e98986 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -20,6 +20,7 @@ import * as Stream from "effect/Stream"; import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; import { + DEFAULT_WORKTREE_PATH_TEMPLATE, GitCommandError, type ReviewDiffPreviewInput, type ReviewDiffPreviewSource, @@ -2267,7 +2268,16 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const targetBranch = input.newRefName ?? input.refName; const sanitizedBranch = targetBranch.replace(/\//g, "-"); const repoName = path.basename(input.cwd); - const worktreePath = input.path ?? path.join(worktreesDir, repoName, sanitizedBranch); + const worktreePath = + input.path ?? + path.resolve( + input.cwd, + (input.pathTemplate ?? DEFAULT_WORKTREE_PATH_TEMPLATE) + .replaceAll("{worktreesDir}", worktreesDir) + .replaceAll("{repoRoot}", input.cwd) + .replaceAll("{repoName}", repoName) + .replaceAll("{branch}", sanitizedBranch), + ); const args = input.newRefName ? ["worktree", "add", "-b", input.newRefName, worktreePath, input.refName] : ["worktree", "add", worktreePath, input.refName]; diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 2a8be25a728..9ee395bff66 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -11,6 +11,7 @@ import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import { DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL, + DEFAULT_WORKTREE_PATH_TEMPLATE, AuthOrchestrationOperateScope, AuthOrchestrationReadScope, AuthReviewWriteScope, @@ -454,6 +455,14 @@ const makeWsRpcLayer = ( }).pipe(Effect.as(DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL)), ), ); + const worktreePathTemplate = serverSettings.getSettings.pipe( + Effect.map((settings) => settings.worktreePathTemplate), + Effect.catch((cause) => + Effect.logWarning("Failed to read worktree path template; using the default", { + detail: cause.message, + }).pipe(Effect.as(DEFAULT_WORKTREE_PATH_TEMPLATE)), + ), + ); const sourceControlRepositories = yield* SourceControlRepositoryService.SourceControlRepositoryService; const bootstrapCredentials = yield* PairingGrantStore.PairingGrantStore; @@ -1028,6 +1037,7 @@ const makeWsRpcLayer = ( newRefName: bootstrap.prepareWorktree.branch, baseRefName: bootstrap.prepareWorktree.baseBranch, path: null, + pathTemplate: yield* worktreePathTemplate, }); targetWorktreePath = worktree.worktree.path; yield* orchestrationEngine.dispatch({ @@ -1838,7 +1848,12 @@ const makeWsRpcLayer = ( [WS_METHODS.vcsCreateWorktree]: (input) => observeRpcEffect( WS_METHODS.vcsCreateWorktree, - gitWorkflow.createWorktree(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), + worktreePathTemplate.pipe( + Effect.flatMap((pathTemplate) => + gitWorkflow.createWorktree({ ...input, pathTemplate }), + ), + Effect.tap(() => refreshGitStatus(input.cwd)), + ), { "rpc.aggregate": "vcs" }, ), [WS_METHODS.vcsRemoveWorktree]: (input) => diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 310cdd5164c..3756bb16b19 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -436,6 +436,9 @@ export function useSettingsRestore(onRestored?: () => void) { DEFAULT_UNIFIED_SETTINGS.newWorktreesStartFromOrigin ? ["New worktrees start from origin"] : []), + ...(settings.worktreePathTemplate !== DEFAULT_UNIFIED_SETTINGS.worktreePathTemplate + ? ["Worktree path"] + : []), ...(settings.addProjectBaseDirectory !== DEFAULT_UNIFIED_SETTINGS.addProjectBaseDirectory ? ["Add project base directory"] : []), @@ -455,6 +458,7 @@ export function useSettingsRestore(onRestored?: () => void) { settings.addProjectBaseDirectory, settings.defaultThreadEnvMode, settings.newWorktreesStartFromOrigin, + settings.worktreePathTemplate, settings.diffIgnoreWhitespace, settings.glassOpacity, settings.automaticGitFetchInterval, @@ -492,6 +496,7 @@ export function useSettingsRestore(onRestored?: () => void) { automaticGitFetchInterval: DEFAULT_UNIFIED_SETTINGS.automaticGitFetchInterval, defaultThreadEnvMode: DEFAULT_UNIFIED_SETTINGS.defaultThreadEnvMode, newWorktreesStartFromOrigin: DEFAULT_UNIFIED_SETTINGS.newWorktreesStartFromOrigin, + worktreePathTemplate: DEFAULT_UNIFIED_SETTINGS.worktreePathTemplate, addProjectBaseDirectory: DEFAULT_UNIFIED_SETTINGS.addProjectBaseDirectory, confirmThreadArchive: DEFAULT_UNIFIED_SETTINGS.confirmThreadArchive, confirmThreadDelete: DEFAULT_UNIFIED_SETTINGS.confirmThreadDelete, @@ -918,6 +923,38 @@ export function GeneralSettingsPanel() { /> ) : null} + + updateSettings({ + worktreePathTemplate: DEFAULT_UNIFIED_SETTINGS.worktreePathTemplate, + }) + } + /> + ) : null + } + control={ + { + const trimmed = next.trim(); + if (trimmed.length > 0) { + updateSettings({ worktreePathTemplate: trimmed }); + } + }} + placeholder="{repoRoot}/.worktrees/{branch}" + spellCheck={false} + aria-label="Worktree path template" + /> + } + /> + { }); }); +describe("ServerSettings worktree path template", () => { + it("defaults to the existing centralized worktree layout", () => { + expect(DEFAULT_SERVER_SETTINGS.worktreePathTemplate).toBe("{worktreesDir}/{repoName}/{branch}"); + }); +}); + describe("ServerSettingsPatch string normalization", () => { it("trims string settings while decoding patches", () => { const patch = decodeServerSettingsPatch({ addProjectBaseDirectory: " ~/Development ", + worktreePathTemplate: " {repoRoot}/.worktrees/{branch} ", textGenerationModelSelection: { model: " gpt-5.4-mini " }, observability: { otlpTracesUrl: " http://localhost:4318/v1/traces ", @@ -218,6 +225,7 @@ describe("ServerSettingsPatch string normalization", () => { }); expect(patch.addProjectBaseDirectory).toBe("~/Development"); + expect(patch.worktreePathTemplate).toBe("{repoRoot}/.worktrees/{branch}"); expect(patch.textGenerationModelSelection?.model).toBe("gpt-5.4-mini"); expect(patch.observability?.otlpTracesUrl).toBe("http://localhost:4318/v1/traces"); expect(patch.providers?.codex?.binaryPath).toBe("/opt/homebrew/bin/codex"); @@ -239,6 +247,7 @@ describe("ServerSettingsPatch string normalization", () => { const encoded = encodeServerSettings({ ...defaultSettings, addProjectBaseDirectory: " ~/Development ", + worktreePathTemplate: " {repoRoot}/.worktrees/{branch} ", providers: { ...defaultSettings.providers, codex: { @@ -250,6 +259,7 @@ describe("ServerSettingsPatch string normalization", () => { }); expect(encoded.addProjectBaseDirectory).toBe("~/Development"); + expect(encoded.worktreePathTemplate).toBe("{repoRoot}/.worktrees/{branch}"); expect(encoded.providers?.codex?.binaryPath).toBe("/opt/homebrew/bin/codex"); expect(encoded.providers?.codex?.launchArgs).toBe("--strict-config"); }); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 4c6306514f6..921ee4a6387 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -414,6 +414,7 @@ export const SourceControlWritingStyleSettings = Schema.Struct({ export type SourceControlWritingStyleSettings = typeof SourceControlWritingStyleSettings.Type; export const DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL = Duration.seconds(30); +export const DEFAULT_WORKTREE_PATH_TEMPLATE = "{worktreesDir}/{repoName}/{branch}"; export const ServerSettings = Schema.Struct({ enableAssistantStreaming: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), @@ -429,6 +430,9 @@ export const ServerSettings = Schema.Struct({ newWorktreesStartFromOrigin: Schema.Boolean.pipe( Schema.withDecodingDefault(Effect.succeed(true)), ), + worktreePathTemplate: TrimmedNonEmptyString.pipe( + Schema.withDecodingDefault(Effect.succeed(DEFAULT_WORKTREE_PATH_TEMPLATE)), + ), addProjectBaseDirectory: TrimmedString.pipe(Schema.withDecodingDefault(Effect.succeed(""))), textGenerationModelSelection: ModelSelection.pipe( Schema.withDecodingDefault( @@ -567,6 +571,7 @@ export const ServerSettingsPatch = Schema.Struct({ automaticGitFetchInterval: Schema.optionalKey(Schema.DurationFromMillis), defaultThreadEnvMode: Schema.optionalKey(ThreadEnvMode), newWorktreesStartFromOrigin: Schema.optionalKey(Schema.Boolean), + worktreePathTemplate: Schema.optionalKey(TrimmedNonEmptyString), addProjectBaseDirectory: Schema.optionalKey(TrimmedString), textGenerationModelSelection: Schema.optionalKey(ModelSelectionPatch), sourceControlWritingStyle: Schema.optionalKey( From 7a05cffc46fecfc0534dbea14737601db388bb71 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 01:58:51 -0700 Subject: [PATCH 2/9] Address configurable worktree review paths --- apps/server/src/review/ReviewService.test.ts | 39 ++++++++++++ apps/server/src/review/ReviewService.ts | 26 +++++++- apps/server/src/server.test.ts | 20 +++---- apps/server/src/vcs/GitVcsDriverCore.test.ts | 18 ++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 17 +++--- apps/server/src/vcs/worktreePathTemplate.ts | 62 ++++++++++++++++++++ 6 files changed, 161 insertions(+), 21 deletions(-) create mode 100644 apps/server/src/vcs/worktreePathTemplate.ts diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index 839eb73b2bb..50620920988 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -3,9 +3,11 @@ 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 Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; import { ServerConfig } from "../config.ts"; +import * as ServerSettings from "../serverSettings.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as ReviewService from "./ReviewService.ts"; @@ -13,6 +15,7 @@ import * as ReviewService from "./ReviewService.ts"; function makeLayer(input: { readonly workspaceRoot: string; readonly baseDir: string; + readonly worktreePathTemplate?: string; readonly detectCalls?: Array<{ readonly cwd: string }>; }) { return ReviewService.layer.pipe( @@ -28,6 +31,11 @@ function makeLayer(input: { }), ), Layer.provide(Layer.mock(GitVcsDriver.GitVcsDriver)({})), + Layer.provide( + ServerSettings.ServerSettingsService.layerTest( + input.worktreePathTemplate ? { worktreePathTemplate: input.worktreePathTemplate } : {}, + ), + ), Layer.provide(ServerConfig.layerTest(input.workspaceRoot, input.baseDir)), Layer.provideMerge(NodeServices.layer), ); @@ -75,6 +83,37 @@ describe("ReviewService", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + it.effect("allows diff preview cwd matching the configured worktree template", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const repositoryRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-repo-" }); + const worktreeRoot = path.join(repositoryRoot, ".worktrees", "feature-local"); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* fs.makeDirectory(worktreeRoot, { recursive: true }); + + const result = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review.getDiffPreview({ cwd: worktreeRoot }); + }).pipe( + Effect.provide( + makeLayer({ + workspaceRoot, + baseDir, + detectCalls, + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", + }), + ), + ); + + assert.strictEqual(result.cwd, worktreeRoot); + assert.deepStrictEqual(result.sources, []); + assert.deepStrictEqual(detectCalls, [{ cwd: worktreeRoot }]); + }).pipe(Effect.provide(NodeServices.layer)), + ); + it.effect("preserves unexpected path-resolution failures", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index db1dc5bc8d2..e928cb79b54 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -6,6 +6,7 @@ import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import { + DEFAULT_WORKTREE_PATH_TEMPLATE, VcsRepositoryDetectionError, VcsUnsupportedOperationError, type ReviewDiffPreviewError, @@ -14,8 +15,10 @@ import { } from "@t3tools/contracts"; import * as ServerConfig from "../config.ts"; +import * as ServerSettings from "../serverSettings.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; +import { matchesWorktreePathTemplate } from "../vcs/worktreePathTemplate.ts"; export class ReviewService extends Context.Service< ReviewService, @@ -30,6 +33,7 @@ export const make = Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; + const serverSettings = yield* ServerSettings.ServerSettingsService; const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry; const git = yield* GitVcsDriver.GitVcsDriver; @@ -66,7 +70,27 @@ export const make = Effect.gen(function* () { canonicalizePath(config.worktreesDir), ]); - if (isWithinRoot(candidate, workspaceRoot) || isWithinRoot(candidate, worktreesRoot)) { + const worktreePathTemplate = yield* serverSettings.getSettings.pipe( + Effect.map((settings) => settings.worktreePathTemplate), + Effect.catch((cause) => + Effect.logWarning("Failed to read worktree path template for review validation", { + cause, + }).pipe(Effect.as(DEFAULT_WORKTREE_PATH_TEMPLATE)), + ), + ); + const matchesConfiguredWorktreePath = matchesWorktreePathTemplate(path, { + candidate, + cwd: workspaceRoot, + worktreesDir: worktreesRoot, + template: worktreePathTemplate, + branch: "__configured_branch__", + }); + + if ( + isWithinRoot(candidate, workspaceRoot) || + isWithinRoot(candidate, worktreesRoot) || + matchesConfiguredWorktreePath + ) { return; } diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 1dfea606b69..d79602a5f91 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -493,6 +493,14 @@ const buildAppUnderTest = (options?: { const gitManagerLayer = Layer.mock(GitManager.GitManager)({ ...options?.layers?.gitManager, }); + const serverSettingsLayer = Layer.mock(ServerSettings.ServerSettingsService)({ + start: Effect.void, + ready: Effect.void, + getSettings: Effect.succeed(DEFAULT_SERVER_SETTINGS), + updateSettings: () => Effect.succeed(DEFAULT_SERVER_SETTINGS), + streamChanges: Stream.empty, + ...options?.layers?.serverSettings, + }); const workspaceEntriesLayer = WorkspaceEntries.layer.pipe( Layer.provide(WorkspacePaths.layer), Layer.provideMerge(vcsDriverRegistryLayer), @@ -524,6 +532,7 @@ const buildAppUnderTest = (options?: { : ReviewService.layer.pipe( Layer.provideMerge(gitVcsDriverLayer), Layer.provide(vcsDriverRegistryLayer), + Layer.provide(serverSettingsLayer), ); const vcsStatusBroadcasterLayer = options?.layers?.vcsStatusBroadcaster ? Layer.mock(VcsStatusBroadcaster.VcsStatusBroadcaster)({ @@ -559,16 +568,7 @@ const buildAppUnderTest = (options?: { ...options?.layers?.providerRegistry, }), ), - Layer.provide( - Layer.mock(ServerSettings.ServerSettingsService)({ - start: Effect.void, - ready: Effect.void, - getSettings: Effect.succeed(DEFAULT_SERVER_SETTINGS), - updateSettings: () => Effect.succeed(DEFAULT_SERVER_SETTINGS), - streamChanges: Stream.empty, - ...options?.layers?.serverSettings, - }), - ), + Layer.provide(serverSettingsLayer), Layer.provide( Layer.mock(ExternalLauncher.ExternalLauncher)({ resolveAvailableEditors: () => Effect.succeed([]), diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index c3d7e40340e..df61e8a37bf 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -14,6 +14,7 @@ import { GitCommandError } from "@t3tools/contracts"; import { ServerConfig } from "../config.ts"; import { splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; +import { resolveWorktreePathTemplate } from "./worktreePathTemplate.ts"; const ServerConfigLayer = ServerConfig.layerTest(process.cwd(), { prefix: "t3-git-vcs-driver-test-", @@ -95,6 +96,23 @@ const initRepoWithCommit = ( return { initialBranch }; }); +it.effect("resolves repoRoot templates from an absolute form of a relative cwd", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const relativeCwd = path.join("repos", "example"); + + assert.equal( + resolveWorktreePathTemplate(path, { + cwd: relativeCwd, + worktreesDir: path.resolve("central-worktrees"), + template: "{repoRoot}/.worktrees/{branch}", + branch: "feature/relative-cwd", + }), + path.resolve(relativeCwd, ".worktrees", "feature-relative-cwd"), + ); + }).pipe(Effect.provide(NodeServices.layer)), +); + it.effect("uses stable diagnostics for every parsed non-repository command", () => { const commands: Array<{ readonly args: ReadonlyArray; readonly lcAll?: string }> = []; const spawner = ChildProcessSpawner.make((command) => diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index da2b7e98986..a0c9fc87114 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -37,6 +37,7 @@ import { parseRemoteRefWithRemoteNames, } from "../git/remoteRefs.ts"; import { ServerConfig } from "../config.ts"; +import { resolveWorktreePathTemplate } from "./worktreePathTemplate.ts"; const DEFAULT_TIMEOUT_MS = 30_000; const DEFAULT_MAX_OUTPUT_BYTES = 1_000_000; @@ -2266,18 +2267,14 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "createWorktree", )(function* (input) { const targetBranch = input.newRefName ?? input.refName; - const sanitizedBranch = targetBranch.replace(/\//g, "-"); - const repoName = path.basename(input.cwd); const worktreePath = input.path ?? - path.resolve( - input.cwd, - (input.pathTemplate ?? DEFAULT_WORKTREE_PATH_TEMPLATE) - .replaceAll("{worktreesDir}", worktreesDir) - .replaceAll("{repoRoot}", input.cwd) - .replaceAll("{repoName}", repoName) - .replaceAll("{branch}", sanitizedBranch), - ); + resolveWorktreePathTemplate(path, { + cwd: input.cwd, + worktreesDir, + template: input.pathTemplate ?? DEFAULT_WORKTREE_PATH_TEMPLATE, + branch: targetBranch, + }); const args = input.newRefName ? ["worktree", "add", "-b", input.newRefName, worktreePath, input.refName] : ["worktree", "add", worktreePath, input.refName]; diff --git a/apps/server/src/vcs/worktreePathTemplate.ts b/apps/server/src/vcs/worktreePathTemplate.ts new file mode 100644 index 00000000000..f83719c4255 --- /dev/null +++ b/apps/server/src/vcs/worktreePathTemplate.ts @@ -0,0 +1,62 @@ +import type * as Path from "effect/Path"; + +interface WorktreePathTemplateInput { + readonly cwd: string; + readonly worktreesDir: string; + readonly template: string; + readonly branch: string; +} + +const escapeRegex = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + +const literalPathPattern = (value: string) => + value + .split(/[\\/]+/) + .map(escapeRegex) + .join("[\\\\/]+"); + +export function resolveWorktreePathTemplate( + path: Path.Path, + input: WorktreePathTemplateInput, +): string { + const repoRoot = path.resolve(input.cwd); + const branch = input.branch.replace(/\//g, "-"); + const expanded = input.template + .replaceAll("{worktreesDir}", path.resolve(input.worktreesDir)) + .replaceAll("{repoRoot}", repoRoot) + .replaceAll("{repoName}", path.basename(repoRoot)) + .replaceAll("{branch}", branch); + + return path.resolve(repoRoot, expanded); +} + +export function matchesWorktreePathTemplate( + path: Path.Path, + input: WorktreePathTemplateInput & { readonly candidate: string }, +): boolean { + const repoRootMarker = path.resolve( + path.sep, + "__t3_worktree_repo_parent__", + "__t3_worktree_repo_name__", + ); + const branchMarker = "__t3_worktree_branch__"; + const resolvedTemplate = resolveWorktreePathTemplate(path, { + ...input, + cwd: repoRootMarker, + branch: branchMarker, + }) + .replaceAll(repoRootMarker, "{repoRoot}") + .replaceAll("__t3_worktree_repo_name__", "{repoName}") + .replaceAll(branchMarker, "{branch}"); + + const pattern = resolvedTemplate + .split(/(\{repoRoot\}|\{repoName\}|\{branch\})/) + .map((part) => { + if (part === "{repoRoot}") return ".+?"; + if (part === "{repoName}" || part === "{branch}") return "[^\\\\/]+"; + return literalPathPattern(part); + }) + .join(""); + + return new RegExp(`^${pattern}$`).test(path.resolve(input.candidate)); +} From e434351273780f695f5abef424c000dd38fe1392 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 02:10:02 -0700 Subject: [PATCH 3/9] Restrict configured worktree review paths --- apps/server/src/review/ReviewService.test.ts | 45 +++++++++++++++++++- apps/server/src/review/ReviewService.ts | 22 +++++----- apps/server/src/vcs/GitVcsDriverCore.test.ts | 18 ++++++++ apps/server/src/vcs/worktreePathTemplate.ts | 41 +++++++----------- apps/server/src/ws.ts | 21 +++++++-- 5 files changed, 108 insertions(+), 39 deletions(-) diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index 50620920988..10d655272d6 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -96,7 +96,10 @@ describe("ReviewService", () => { const result = yield* Effect.gen(function* () { const review = yield* ReviewService.ReviewService; - return yield* review.getDiffPreview({ cwd: worktreeRoot }); + return yield* review.getDiffPreview({ + cwd: worktreeRoot, + repositoryRoots: [repositoryRoot], + }); }).pipe( Effect.provide( makeLayer({ @@ -114,6 +117,46 @@ describe("ReviewService", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + it.effect("rejects configured-template paths outside known repository roots", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const knownRepositoryRoot = yield* fs.makeTempDirectoryScoped({ + prefix: "t3-review-known-repo-", + }); + const unknownRepositoryRoot = yield* fs.makeTempDirectoryScoped({ + prefix: "t3-review-unknown-repo-", + }); + const unknownWorktreeRoot = path.join(unknownRepositoryRoot, ".worktrees", "feature-local"); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* fs.makeDirectory(unknownWorktreeRoot, { recursive: true }); + + const error = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review + .getDiffPreview({ + cwd: unknownWorktreeRoot, + repositoryRoots: [knownRepositoryRoot], + }) + .pipe(Effect.flip); + }).pipe( + Effect.provide( + makeLayer({ + workspaceRoot, + baseDir, + detectCalls, + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", + }), + ), + ); + + assert.strictEqual(error._tag, "VcsRepositoryDetectionError"); + assert.deepStrictEqual(detectCalls, []); + }).pipe(Effect.provide(NodeServices.layer)), + ); + it.effect("preserves unexpected path-resolution failures", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index e928cb79b54..03b61f575c0 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -24,7 +24,7 @@ export class ReviewService extends Context.Service< ReviewService, { readonly getDiffPreview: ( - input: ReviewDiffPreviewInput, + input: ReviewDiffPreviewInput & { readonly repositoryRoots?: ReadonlyArray }, ) => Effect.Effect; } >()("t3/review/ReviewService") {} @@ -62,8 +62,9 @@ export const make = Effect.gen(function* () { }; const assertWorkspaceBoundCwd = Effect.fn("ReviewService.assertWorkspaceBoundCwd")(function* ( - cwd: string, + input: ReviewDiffPreviewInput & { readonly repositoryRoots?: ReadonlyArray }, ) { + const { cwd } = input; const [candidate, workspaceRoot, worktreesRoot] = yield* Effect.all([ canonicalizePath(cwd), canonicalizePath(config.cwd), @@ -78,13 +79,14 @@ export const make = Effect.gen(function* () { }).pipe(Effect.as(DEFAULT_WORKTREE_PATH_TEMPLATE)), ), ); - const matchesConfiguredWorktreePath = matchesWorktreePathTemplate(path, { - candidate, - cwd: workspaceRoot, - worktreesDir: worktreesRoot, - template: worktreePathTemplate, - branch: "__configured_branch__", - }); + const matchesConfiguredWorktreePath = (input.repositoryRoots ?? []).some((repositoryRoot) => + matchesWorktreePathTemplate(path, { + candidate, + cwd: repositoryRoot, + worktreesDir: worktreesRoot, + template: worktreePathTemplate, + }), + ); if ( isWithinRoot(candidate, workspaceRoot) || @@ -104,7 +106,7 @@ export const make = Effect.gen(function* () { const getDiffPreview: ReviewService["Service"]["getDiffPreview"] = Effect.fn( "ReviewService.getDiffPreview", )(function* (input) { - yield* assertWorkspaceBoundCwd(input.cwd); + yield* assertWorkspaceBoundCwd(input); const handle = yield* vcsRegistry.detect({ cwd: input.cwd, requestedKind: "auto" }); if (!handle) { diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index df61e8a37bf..f0d224f4490 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -113,6 +113,24 @@ it.effect("resolves repoRoot templates from an absolute form of a relative cwd", }).pipe(Effect.provide(NodeServices.layer)), ); +it.effect("expands path template placeholders in a single pass", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const cwd = path.resolve("repos", "example"); + const worktreesDir = path.resolve("central-worktrees", "{branch}"); + + assert.equal( + resolveWorktreePathTemplate(path, { + cwd, + worktreesDir, + template: "{worktreesDir}/{repoName}/{branch}", + branch: "feature/single-pass", + }), + path.join(worktreesDir, "example", "feature-single-pass"), + ); + }).pipe(Effect.provide(NodeServices.layer)), +); + it.effect("uses stable diagnostics for every parsed non-repository command", () => { const commands: Array<{ readonly args: ReadonlyArray; readonly lcAll?: string }> = []; const spawner = ChildProcessSpawner.make((command) => diff --git a/apps/server/src/vcs/worktreePathTemplate.ts b/apps/server/src/vcs/worktreePathTemplate.ts index f83719c4255..146f275963e 100644 --- a/apps/server/src/vcs/worktreePathTemplate.ts +++ b/apps/server/src/vcs/worktreePathTemplate.ts @@ -7,6 +7,8 @@ interface WorktreePathTemplateInput { readonly branch: string; } +const WORKTREE_PATH_PLACEHOLDER = /\{(worktreesDir|repoRoot|repoName|branch)\}/g; + const escapeRegex = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const literalPathPattern = (value: string) => @@ -21,42 +23,31 @@ export function resolveWorktreePathTemplate( ): string { const repoRoot = path.resolve(input.cwd); const branch = input.branch.replace(/\//g, "-"); - const expanded = input.template - .replaceAll("{worktreesDir}", path.resolve(input.worktreesDir)) - .replaceAll("{repoRoot}", repoRoot) - .replaceAll("{repoName}", path.basename(repoRoot)) - .replaceAll("{branch}", branch); + const placeholders = { + worktreesDir: path.resolve(input.worktreesDir), + repoRoot, + repoName: path.basename(repoRoot), + branch, + } as const; + const expanded = input.template.replace( + WORKTREE_PATH_PLACEHOLDER, + (_placeholder, name: keyof typeof placeholders) => placeholders[name], + ); return path.resolve(repoRoot, expanded); } export function matchesWorktreePathTemplate( path: Path.Path, - input: WorktreePathTemplateInput & { readonly candidate: string }, + input: Omit & { readonly candidate: string }, ): boolean { - const repoRootMarker = path.resolve( - path.sep, - "__t3_worktree_repo_parent__", - "__t3_worktree_repo_name__", - ); const branchMarker = "__t3_worktree_branch__"; const resolvedTemplate = resolveWorktreePathTemplate(path, { ...input, - cwd: repoRootMarker, branch: branchMarker, - }) - .replaceAll(repoRootMarker, "{repoRoot}") - .replaceAll("__t3_worktree_repo_name__", "{repoName}") - .replaceAll(branchMarker, "{branch}"); - - const pattern = resolvedTemplate - .split(/(\{repoRoot\}|\{repoName\}|\{branch\})/) - .map((part) => { - if (part === "{repoRoot}") return ".+?"; - if (part === "{repoName}" || part === "{branch}") return "[^\\\\/]+"; - return literalPathPattern(part); - }) - .join(""); + }); + + const pattern = resolvedTemplate.split(branchMarker).map(literalPathPattern).join("[^\\\\/]+"); return new RegExp(`^${pattern}$`).test(path.resolve(input.candidate)); } diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 9ee395bff66..05d60c85698 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1883,9 +1883,24 @@ const makeWsRpcLayer = ( { "rpc.aggregate": "vcs" }, ), [WS_METHODS.reviewGetDiffPreview]: (input) => - observeRpcEffect(WS_METHODS.reviewGetDiffPreview, review.getDiffPreview(input), { - "rpc.aggregate": "review", - }), + observeRpcEffect( + WS_METHODS.reviewGetDiffPreview, + Effect.gen(function* () { + const repositoryRoots = yield* projectionSnapshotQuery.getShellSnapshot().pipe( + Effect.map((snapshot) => snapshot.projects.map((project) => project.workspaceRoot)), + Effect.catch((cause) => + Effect.logWarning( + "Failed to read project roots for review workspace validation", + { cause }, + ).pipe(Effect.as([])), + ), + ); + return yield* review.getDiffPreview({ ...input, repositoryRoots }); + }), + { + "rpc.aggregate": "review", + }, + ), [WS_METHODS.terminalOpen]: (input) => observeRpcEffect(WS_METHODS.terminalOpen, terminalManager.open(input), { "rpc.aggregate": "terminal", From c5dedb1603ab994c9932b3a6ea2e511bf07adfad Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 02:16:06 -0700 Subject: [PATCH 4/9] Canonicalize configured review roots --- apps/server/src/review/ReviewService.test.ts | 7 ++++++- apps/server/src/review/ReviewService.ts | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index 10d655272d6..a3458b4ea71 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -90,15 +90,20 @@ describe("ReviewService", () => { const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" }); const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); const repositoryRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-repo-" }); + const repositoryAliasesRoot = yield* fs.makeTempDirectoryScoped({ + prefix: "t3-review-repo-aliases-", + }); + const repositoryRootAlias = path.join(repositoryAliasesRoot, "repo"); const worktreeRoot = path.join(repositoryRoot, ".worktrees", "feature-local"); const detectCalls: Array<{ readonly cwd: string }> = []; + yield* fs.symlink(repositoryRoot, repositoryRootAlias); yield* fs.makeDirectory(worktreeRoot, { recursive: true }); const result = yield* Effect.gen(function* () { const review = yield* ReviewService.ReviewService; return yield* review.getDiffPreview({ cwd: worktreeRoot, - repositoryRoots: [repositoryRoot], + repositoryRoots: [repositoryRootAlias], }); }).pipe( Effect.provide( diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index 03b61f575c0..ecf65fc748d 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -79,7 +79,10 @@ export const make = Effect.gen(function* () { }).pipe(Effect.as(DEFAULT_WORKTREE_PATH_TEMPLATE)), ), ); - const matchesConfiguredWorktreePath = (input.repositoryRoots ?? []).some((repositoryRoot) => + const repositoryRoots = yield* Effect.forEach(input.repositoryRoots ?? [], canonicalizePath, { + concurrency: "unbounded", + }); + const matchesConfiguredWorktreePath = repositoryRoots.some((repositoryRoot) => matchesWorktreePathTemplate(path, { candidate, cwd: repositoryRoot, From fb080a6c58fa205d5aa724a63dd533c93f1ba857 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 02:24:53 -0700 Subject: [PATCH 5/9] Preserve configured worktree path semantics --- apps/server/src/review/ReviewService.test.ts | 78 ++++++++++++++++++++ apps/server/src/review/ReviewService.ts | 32 +++++--- apps/server/src/vcs/worktreePathTemplate.ts | 6 +- 3 files changed, 105 insertions(+), 11 deletions(-) diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index a3458b4ea71..0cc9c79b7d4 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -122,6 +122,84 @@ describe("ReviewService", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + it.effect("preserves lexical repo names when matching canonical worktree paths", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const repositoryParent = yield* fs.makeTempDirectoryScoped({ + prefix: "t3-review-repo-parent-", + }); + const repositoryRoot = path.join(repositoryParent, "physical-repo-name"); + const repositoryRootAlias = path.join(repositoryParent, "lexical-repo-name"); + const worktreeRoot = path.join( + repositoryParent, + "lexical-repo-name-worktrees", + "feature-local", + ); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* fs.makeDirectory(repositoryRoot); + yield* fs.symlink(repositoryRoot, repositoryRootAlias); + yield* fs.makeDirectory(worktreeRoot, { recursive: true }); + + const result = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review.getDiffPreview({ + cwd: worktreeRoot, + repositoryRoots: [repositoryRootAlias], + }); + }).pipe( + Effect.provide( + makeLayer({ + workspaceRoot, + baseDir, + detectCalls, + worktreePathTemplate: "{repoRoot}/../{repoName}-worktrees/{branch}", + }), + ), + ); + + assert.strictEqual(result.cwd, worktreeRoot); + assert.deepStrictEqual(result.sources, []); + assert.deepStrictEqual(detectCalls, [{ cwd: worktreeRoot }]); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("skips invalid unrelated repository roots", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const repositoryRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-repo-" }); + const worktreeRoot = path.join(repositoryRoot, ".worktrees", "feature-local"); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* fs.makeDirectory(worktreeRoot, { recursive: true }); + + const result = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review.getDiffPreview({ + cwd: worktreeRoot, + repositoryRoots: [`${repositoryRoot}\0invalid`, repositoryRoot], + }); + }).pipe( + Effect.provide( + makeLayer({ + workspaceRoot, + baseDir, + detectCalls, + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", + }), + ), + ); + + assert.strictEqual(result.cwd, worktreeRoot); + assert.deepStrictEqual(result.sources, []); + assert.deepStrictEqual(detectCalls, [{ cwd: worktreeRoot }]); + }).pipe(Effect.provide(NodeServices.layer)), + ); + it.effect("rejects configured-template paths outside known repository roots", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index ecf65fc748d..ec49495b12f 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -79,16 +79,30 @@ export const make = Effect.gen(function* () { }).pipe(Effect.as(DEFAULT_WORKTREE_PATH_TEMPLATE)), ), ); - const repositoryRoots = yield* Effect.forEach(input.repositoryRoots ?? [], canonicalizePath, { - concurrency: "unbounded", - }); + const repositoryRoots = yield* Effect.forEach( + input.repositoryRoots ?? [], + (repositoryRoot) => + canonicalizePath(repositoryRoot).pipe( + Effect.map((resolvedRepoRoot) => ({ repositoryRoot, resolvedRepoRoot })), + Effect.catch((cause) => + Effect.logWarning("Skipping repository root that could not be resolved", { + cause, + repositoryRoot, + }).pipe(Effect.as(null)), + ), + ), + { concurrency: "unbounded" }, + ); const matchesConfiguredWorktreePath = repositoryRoots.some((repositoryRoot) => - matchesWorktreePathTemplate(path, { - candidate, - cwd: repositoryRoot, - worktreesDir: worktreesRoot, - template: worktreePathTemplate, - }), + repositoryRoot === null + ? false + : matchesWorktreePathTemplate(path, { + candidate, + cwd: repositoryRoot.repositoryRoot, + resolvedRepoRoot: repositoryRoot.resolvedRepoRoot, + worktreesDir: worktreesRoot, + template: worktreePathTemplate, + }), ); if ( diff --git a/apps/server/src/vcs/worktreePathTemplate.ts b/apps/server/src/vcs/worktreePathTemplate.ts index 146f275963e..1d914387628 100644 --- a/apps/server/src/vcs/worktreePathTemplate.ts +++ b/apps/server/src/vcs/worktreePathTemplate.ts @@ -2,6 +2,7 @@ import type * as Path from "effect/Path"; interface WorktreePathTemplateInput { readonly cwd: string; + readonly resolvedRepoRoot?: string; readonly worktreesDir: string; readonly template: string; readonly branch: string; @@ -21,12 +22,13 @@ export function resolveWorktreePathTemplate( path: Path.Path, input: WorktreePathTemplateInput, ): string { - const repoRoot = path.resolve(input.cwd); + const lexicalRepoRoot = path.resolve(input.cwd); + const repoRoot = path.resolve(input.resolvedRepoRoot ?? lexicalRepoRoot); const branch = input.branch.replace(/\//g, "-"); const placeholders = { worktreesDir: path.resolve(input.worktreesDir), repoRoot, - repoName: path.basename(repoRoot), + repoName: path.basename(lexicalRepoRoot), branch, } as const; const expanded = input.template.replace( From 42f6450e7e96378391c2c2f90d49e41c38ec1de6 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 12:14:30 -0700 Subject: [PATCH 6/9] Surface review project snapshot failures --- apps/server/src/server.test.ts | 48 ++++++++++++++++++++++++++++++++++ apps/server/src/ws.ts | 15 +++++++---- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index d79602a5f91..135971eceb9 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -5241,6 +5241,54 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + it.effect("surfaces project snapshot failures before review workspace validation", () => + Effect.gen(function* () { + const projectionError = new PersistenceSqlError({ + operation: "ProjectionSnapshotQuery.getShellSnapshot:test-review", + detail: "failed to read review project roots", + }); + let reviewCalls = 0; + yield* buildAppUnderTest({ + layers: { + projectionSnapshotQuery: { + getShellSnapshot: () => Effect.fail(projectionError), + }, + reviewService: { + getDiffPreview: () => + Effect.sync(() => { + reviewCalls += 1; + return { + cwd: "/tmp/repo", + generatedAt: TEST_EPOCH, + sources: [], + }; + }), + }, + }, + }); + + const wsUrl = yield* getWsServerUrl("/ws"); + const result = yield* Effect.scoped( + withWsRpcClient(wsUrl, (client) => + client[WS_METHODS.reviewGetDiffPreview]({ + cwd: "/tmp/repository-worktrees/feature", + }), + ).pipe(Effect.result), + ); + + if (result._tag !== "Failure" || result.failure._tag !== "VcsRepositoryDetectionError") { + assert.fail("Expected a VcsRepositoryDetectionError"); + } + const detectionError = result.failure; + assert.equal(detectionError.operation, "review.getDiffPreview"); + assert.equal(detectionError.cwd, "/tmp/repository-worktrees/feature"); + assert.include(detectionError.detail, "Failed to load project roots"); + assert.instanceOf(detectionError.cause, Error); + assert.include(detectionError.cause.message, projectionError.message); + assert.equal(reviewCalls, 0); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + it.effect("routes websocket rpc git.runStackedAction errors after refreshing git status", () => Effect.gen(function* () { const gitError = new GitCommandError({ diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 05d60c85698..6bd1610932e 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -58,6 +58,7 @@ import { type TerminalError, type TerminalEvent, type TerminalMetadataStreamEvent, + VcsRepositoryDetectionError, WS_METHODS, WsRpcGroup, } from "@t3tools/contracts"; @@ -1888,11 +1889,15 @@ const makeWsRpcLayer = ( Effect.gen(function* () { const repositoryRoots = yield* projectionSnapshotQuery.getShellSnapshot().pipe( Effect.map((snapshot) => snapshot.projects.map((project) => project.workspaceRoot)), - Effect.catch((cause) => - Effect.logWarning( - "Failed to read project roots for review workspace validation", - { cause }, - ).pipe(Effect.as([])), + Effect.mapError( + (cause) => + new VcsRepositoryDetectionError({ + operation: "review.getDiffPreview", + cwd: input.cwd, + detail: + "Failed to load project roots required to validate the review workspace.", + cause, + }), ), ); return yield* review.getDiffPreview({ ...input, repositoryRoots }); From b30525952662d0d7addede3f442ff600956cc1ef Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Fri, 24 Jul 2026 17:37:39 -0700 Subject: [PATCH 7/9] fix(server): tighten configurable worktree validation --- .../checkpointing/CheckpointDiffQuery.test.ts | 5 +++ .../Layers/OrchestrationEngine.test.ts | 1 + .../Layers/ProjectionSnapshotQuery.test.ts | 2 + .../Layers/ProjectionSnapshotQuery.ts | 28 ++++++++++++ .../Services/ProjectionSnapshotQuery.ts | 8 ++++ .../project/ProjectSetupScriptRunner.test.ts | 1 + .../Layers/ProviderSessionReaper.test.ts | 1 + apps/server/src/server.test.ts | 45 +++++++++++++++++-- apps/server/src/serverRuntimeStartup.test.ts | 4 ++ apps/server/src/vcs/GitVcsDriverCore.test.ts | 44 +++++++++++++++++- apps/server/src/vcs/GitVcsDriverCore.ts | 27 ++++++++--- apps/server/src/ws.ts | 27 +++++------ 12 files changed, 170 insertions(+), 23 deletions(-) diff --git a/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts b/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts index 8e0e5fb74d5..c8e6641a16a 100644 --- a/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts +++ b/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts @@ -85,6 +85,7 @@ describe("CheckpointDiffQuery.layer", () => { Effect.die("CheckpointDiffQuery should not request archived shell snapshots"), getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -193,6 +194,7 @@ describe("CheckpointDiffQuery.layer", () => { Effect.die("CheckpointDiffQuery should not request archived shell snapshots"), getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -276,6 +278,7 @@ describe("CheckpointDiffQuery.layer", () => { Effect.die("CheckpointDiffQuery should not request archived shell snapshots"), getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -344,6 +347,7 @@ describe("CheckpointDiffQuery.layer", () => { Effect.die("CheckpointDiffQuery should not request archived shell snapshots"), getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -397,6 +401,7 @@ describe("CheckpointDiffQuery.layer", () => { Effect.die("CheckpointDiffQuery should not request archived shell snapshots"), getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), diff --git a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts index 731002ea783..e32944eec1f 100644 --- a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts +++ b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts @@ -195,6 +195,7 @@ describe("OrchestrationEngine", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: projectionSnapshot.snapshotSequence }), getCounts: () => Effect.succeed({ projectCount: 1, threadCount: 1 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts index d4a24a209ad..a0deb0585ec 100644 --- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts +++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts @@ -794,6 +794,8 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => { threadCount: 3, }); + assert.deepEqual(yield* snapshotQuery.getActiveProjectWorkspaceRoots(), ["/tmp/workspace"]); + const project = yield* snapshotQuery.getActiveProjectByWorkspaceRoot("/tmp/workspace"); assert.equal(project._tag, "Some"); if (project._tag === "Some") { diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts index 3d05bef4bdf..b45e0640bb1 100644 --- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts +++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts @@ -108,6 +108,9 @@ const ProjectionCountsRowSchema = Schema.Struct({ projectCount: Schema.Number, threadCount: Schema.Number, }); +const ProjectionWorkspaceRootRowSchema = Schema.Struct({ + workspaceRoot: Schema.String, +}); const WorkspaceRootLookupInput = Schema.Struct({ workspaceRoot: Schema.String, }); @@ -316,6 +319,18 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () { `, }); + const listActiveProjectWorkspaceRootRows = SqlSchema.findAll({ + Request: Schema.Void, + Result: ProjectionWorkspaceRootRowSchema, + execute: () => + sql` + SELECT workspace_root AS "workspaceRoot" + FROM projection_projects + WHERE deleted_at IS NULL + ORDER BY created_at ASC, project_id ASC + `, + }); + const listThreadRows = SqlSchema.findAll({ Request: Schema.Void, Result: ProjectionThreadDbRowSchema, @@ -1737,6 +1752,18 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () { ), ); + const getActiveProjectWorkspaceRoots: ProjectionSnapshotQueryShape["getActiveProjectWorkspaceRoots"] = + () => + listActiveProjectWorkspaceRootRows(undefined).pipe( + Effect.mapError( + toPersistenceSqlOrDecodeError( + "ProjectionSnapshotQuery.getActiveProjectWorkspaceRoots:query", + "ProjectionSnapshotQuery.getActiveProjectWorkspaceRoots:decodeRows", + ), + ), + Effect.map((rows) => rows.map((row) => row.workspaceRoot)), + ); + const getActiveProjectByWorkspaceRoot: ProjectionSnapshotQueryShape["getActiveProjectByWorkspaceRoot"] = (workspaceRoot) => getActiveProjectRowByWorkspaceRoot({ workspaceRoot }).pipe( @@ -2110,6 +2137,7 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () { getArchivedShellSnapshot, getSnapshotSequence, getCounts, + getActiveProjectWorkspaceRoots, getActiveProjectByWorkspaceRoot, getProjectShellById, getFirstActiveThreadIdByProjectId, diff --git a/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts b/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts index 23b291d8778..3e0066f8799 100644 --- a/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts +++ b/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts @@ -108,6 +108,14 @@ export interface ProjectionSnapshotQueryShape { */ readonly getCounts: () => Effect.Effect; + /** + * Read only the workspace roots for active projects. + */ + readonly getActiveProjectWorkspaceRoots: () => Effect.Effect< + ReadonlyArray, + ProjectionRepositoryError + >; + /** * Read the active project for an exact workspace root match. */ diff --git a/apps/server/src/project/ProjectSetupScriptRunner.test.ts b/apps/server/src/project/ProjectSetupScriptRunner.test.ts index 15612908079..1331fbe289b 100644 --- a/apps/server/src/project/ProjectSetupScriptRunner.test.ts +++ b/apps/server/src/project/ProjectSetupScriptRunner.test.ts @@ -32,6 +32,7 @@ const makeProjectionSnapshotQueryLayer = (project: OrchestrationProject) => getArchivedShellSnapshot: () => Effect.die("unused"), getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 1 }), getCounts: () => Effect.die("unused"), + getActiveProjectWorkspaceRoots: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: (workspaceRoot) => Effect.succeed( workspaceRoot === project.workspaceRoot ? Option.some(project) : Option.none(), diff --git a/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts b/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts index 3843c8acbcd..c4d585db265 100644 --- a/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts +++ b/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts @@ -199,6 +199,7 @@ describe("ProviderSessionReaper", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: input.readModel.snapshotSequence }), getCounts: () => Effect.die("unused"), + getActiveProjectWorkspaceRoots: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.die("unused"), getProjectShellById: () => Effect.die("unused"), getFirstActiveThreadIdByProjectId: () => Effect.die("unused"), diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 135971eceb9..7da35cdc849 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -716,6 +716,7 @@ const buildAppUnderTest = (options?: { getThreadDetailById: () => Effect.succeed(Option.none()), getThreadDetailSnapshot: () => Effect.succeed(Option.none()), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), + getActiveProjectWorkspaceRoots: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), getThreadCheckpointContext: () => Effect.succeed(Option.none()), @@ -5241,17 +5242,55 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); - it.effect("surfaces project snapshot failures before review workspace validation", () => + it.effect("loads review project roots without hydrating the shell snapshot", () => + Effect.gen(function* () { + const repositoryRoots = ["/tmp/project-a", "/tmp/project-b"]; + let receivedRepositoryRoots: ReadonlyArray | undefined; + yield* buildAppUnderTest({ + layers: { + projectionSnapshotQuery: { + getShellSnapshot: () => Effect.die("review preview must not hydrate shell snapshots"), + getActiveProjectWorkspaceRoots: () => Effect.succeed(repositoryRoots), + }, + reviewService: { + getDiffPreview: (input) => + Effect.sync(() => { + receivedRepositoryRoots = input.repositoryRoots; + return { + cwd: input.cwd, + generatedAt: TEST_EPOCH, + sources: [], + }; + }), + }, + }, + }); + + const wsUrl = yield* getWsServerUrl("/ws"); + const result = yield* Effect.scoped( + withWsRpcClient(wsUrl, (client) => + client[WS_METHODS.reviewGetDiffPreview]({ + cwd: "/tmp/project-a/.worktrees/feature", + }), + ), + ); + + assert.deepEqual(receivedRepositoryRoots, repositoryRoots); + assert.equal(result.cwd, "/tmp/project-a/.worktrees/feature"); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + + it.effect("surfaces project root query failures before review workspace validation", () => Effect.gen(function* () { const projectionError = new PersistenceSqlError({ - operation: "ProjectionSnapshotQuery.getShellSnapshot:test-review", + operation: "ProjectionSnapshotQuery.getActiveProjectWorkspaceRoots:test-review", detail: "failed to read review project roots", }); let reviewCalls = 0; yield* buildAppUnderTest({ layers: { projectionSnapshotQuery: { - getShellSnapshot: () => Effect.fail(projectionError), + getActiveProjectWorkspaceRoots: () => Effect.fail(projectionError), }, reviewService: { getDiffPreview: () => diff --git a/apps/server/src/serverRuntimeStartup.test.ts b/apps/server/src/serverRuntimeStartup.test.ts index b8102bda9ad..3ddea43313a 100644 --- a/apps/server/src/serverRuntimeStartup.test.ts +++ b/apps/server/src/serverRuntimeStartup.test.ts @@ -89,6 +89,7 @@ it.effect("launchStartupHeartbeat does not block the caller while counts are loa threadCount: 3, }), ), + getActiveProjectWorkspaceRoots: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -140,6 +141,7 @@ it.effect("resolveAutoBootstrapWelcomeTargets returns existing project and threa getArchivedShellSnapshot: () => Effect.die("unused"), getSnapshotSequence: () => Effect.die("unused"), getCounts: () => Effect.die("unused"), + getActiveProjectWorkspaceRoots: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed( Option.some({ @@ -196,6 +198,7 @@ it.effect("resolveAutoBootstrapWelcomeTargets creates a project and thread when getArchivedShellSnapshot: () => Effect.die("unused"), getSnapshotSequence: () => Effect.die("unused"), getCounts: () => Effect.die("unused"), + getActiveProjectWorkspaceRoots: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.die("unused"), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -246,6 +249,7 @@ it.effect("resolveAutoBootstrapWelcomeTargets preserves typed UUID generation fa getArchivedShellSnapshot: () => Effect.die("unused"), getSnapshotSequence: () => Effect.die("unused"), getCounts: () => Effect.die("unused"), + getActiveProjectWorkspaceRoots: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.die("unused"), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index f0d224f4490..1f2903f70b2 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -14,7 +14,10 @@ import { GitCommandError } from "@t3tools/contracts"; import { ServerConfig } from "../config.ts"; import { splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; -import { resolveWorktreePathTemplate } from "./worktreePathTemplate.ts"; +import { + matchesWorktreePathTemplate, + resolveWorktreePathTemplate, +} from "./worktreePathTemplate.ts"; const ServerConfigLayer = ServerConfig.layerTest(process.cwd(), { prefix: "t3-git-vcs-driver-test-", @@ -193,6 +196,45 @@ it.effect("uses the configured repository-local worktree path template", () => }).pipe(Effect.provide(TestLayer)), ); +it.effect("canonicalizes symlinked repository roots before expanding path templates", () => + Effect.gen(function* () { + const tempRoot = yield* makeTmpDir("template-symlinked-root-"); + const pathService = yield* Path.Path; + const fileSystem = yield* FileSystem.FileSystem; + const repositoryRoot = pathService.join(tempRoot, "physical", "repo"); + const repositoryAliasesRoot = pathService.join(tempRoot, "aliases"); + const repositoryRootAlias = pathService.join(repositoryAliasesRoot, "repo"); + yield* fileSystem.makeDirectory(repositoryRoot, { recursive: true }); + yield* fileSystem.makeDirectory(repositoryAliasesRoot); + yield* fileSystem.symlink(repositoryRoot, repositoryRootAlias); + const { initialBranch } = yield* initRepoWithCommit(repositoryRoot); + const driver = yield* GitVcsDriver.GitVcsDriver; + + const created = yield* driver.createWorktree({ + cwd: repositoryRootAlias, + path: null, + refName: initialBranch, + newRefName: "feature/symlinked-root", + pathTemplate: "{repoRoot}/../worktrees/{branch}", + }); + + const expectedPath = pathService.resolve(repositoryRoot, "../worktrees/feature-symlinked-root"); + assert.equal(created.worktree.path, expectedPath); + assert.isTrue( + matchesWorktreePathTemplate(pathService, { + candidate: created.worktree.path, + cwd: repositoryRootAlias, + resolvedRepoRoot: repositoryRoot, + worktreesDir: pathService.join(tempRoot, "central-worktrees"), + template: "{repoRoot}/../worktrees/{branch}", + }), + ); + assert.equal(yield* git(expectedPath, ["branch", "--show-current"]), "feature/symlinked-root"); + + yield* driver.removeWorktree({ cwd: repositoryRootAlias, path: expectedPath }); + }).pipe(Effect.provide(TestLayer)), +); + it.layer(TestLayer)("GitVcsDriver core integration", (it) => { describe("process environment", () => { it.effect("preserves the caller locale for general Git subprocesses", () => diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index a0c9fc87114..71dd1952d0b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -2269,12 +2269,27 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const targetBranch = input.newRefName ?? input.refName; const worktreePath = input.path ?? - resolveWorktreePathTemplate(path, { - cwd: input.cwd, - worktreesDir, - template: input.pathTemplate ?? DEFAULT_WORKTREE_PATH_TEMPLATE, - branch: targetBranch, - }); + (yield* fileSystem.realPath(path.resolve(input.cwd)).pipe( + Effect.map((resolvedRepoRoot) => + resolveWorktreePathTemplate(path, { + cwd: input.cwd, + resolvedRepoRoot, + worktreesDir, + template: input.pathTemplate ?? DEFAULT_WORKTREE_PATH_TEMPLATE, + branch: targetBranch, + }), + ), + Effect.mapError( + (cause) => + new GitCommandError({ + operation: "GitVcsDriver.createWorktree", + command: "git worktree add", + cwd: input.cwd, + detail: "Failed to resolve the repository root before creating a worktree.", + cause, + }), + ), + )); const args = input.newRefName ? ["worktree", "add", "-b", input.newRefName, worktreePath, input.refName] : ["worktree", "add", worktreePath, input.refName]; diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 6bd1610932e..c4a8a15eb1f 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1887,19 +1887,20 @@ const makeWsRpcLayer = ( observeRpcEffect( WS_METHODS.reviewGetDiffPreview, Effect.gen(function* () { - const repositoryRoots = yield* projectionSnapshotQuery.getShellSnapshot().pipe( - Effect.map((snapshot) => snapshot.projects.map((project) => project.workspaceRoot)), - Effect.mapError( - (cause) => - new VcsRepositoryDetectionError({ - operation: "review.getDiffPreview", - cwd: input.cwd, - detail: - "Failed to load project roots required to validate the review workspace.", - cause, - }), - ), - ); + const repositoryRoots = yield* projectionSnapshotQuery + .getActiveProjectWorkspaceRoots() + .pipe( + Effect.mapError( + (cause) => + new VcsRepositoryDetectionError({ + operation: "review.getDiffPreview", + cwd: input.cwd, + detail: + "Failed to load project roots required to validate the review workspace.", + cause, + }), + ), + ); return yield* review.getDiffPreview({ ...input, repositoryRoots }); }), { From 880055380a40d2052896401a4134d2415c4d2b41 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Mon, 27 Jul 2026 18:31:15 -0700 Subject: [PATCH 8/9] fix(server): preserve review access to existing worktrees --- .../checkpointing/CheckpointDiffQuery.test.ts | 5 +++ .../Layers/OrchestrationEngine.test.ts | 1 + .../Layers/ProjectionSnapshotQuery.test.ts | 7 ++-- .../Layers/ProjectionSnapshotQuery.ts | 32 +++++++++++++++++ .../Services/ProjectionSnapshotQuery.ts | 8 +++++ .../project/ProjectSetupScriptRunner.test.ts | 1 + .../Layers/ProviderSessionReaper.test.ts | 1 + apps/server/src/review/ReviewService.test.ts | 36 +++++++++++++++++++ apps/server/src/review/ReviewService.ts | 27 ++++++++++++-- apps/server/src/server.test.ts | 8 ++++- apps/server/src/serverRuntimeStartup.test.ts | 4 +++ apps/server/src/vcs/GitVcsDriverCore.test.ts | 25 +++++++++++++ apps/server/src/vcs/worktreePathTemplate.ts | 12 ++++++- apps/server/src/ws.ts | 35 ++++++++++-------- 14 files changed, 181 insertions(+), 21 deletions(-) diff --git a/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts b/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts index c8e6641a16a..21e7f954d93 100644 --- a/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts +++ b/apps/server/src/checkpointing/CheckpointDiffQuery.test.ts @@ -86,6 +86,7 @@ describe("CheckpointDiffQuery.layer", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -195,6 +196,7 @@ describe("CheckpointDiffQuery.layer", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -279,6 +281,7 @@ describe("CheckpointDiffQuery.layer", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -348,6 +351,7 @@ describe("CheckpointDiffQuery.layer", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -402,6 +406,7 @@ describe("CheckpointDiffQuery.layer", () => { getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 0 }), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), diff --git a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts index e32944eec1f..57e2200959a 100644 --- a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts +++ b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts @@ -196,6 +196,7 @@ describe("OrchestrationEngine", () => { Effect.succeed({ snapshotSequence: projectionSnapshot.snapshotSequence }), getCounts: () => Effect.succeed({ projectCount: 1, threadCount: 1 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts index a0deb0585ec..2f56d39f67b 100644 --- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts +++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts @@ -749,7 +749,7 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => { 'full-access', 'default', NULL, - NULL, + '/tmp/worktree-first', NULL, '2026-03-01T00:00:05.000Z', '2026-03-01T00:00:06.000Z', @@ -779,7 +779,7 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => { 'full-access', 'default', NULL, - NULL, + '/tmp/worktree-deleted', NULL, '2026-03-01T00:00:09.000Z', '2026-03-01T00:00:10.000Z', @@ -795,6 +795,9 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => { }); assert.deepEqual(yield* snapshotQuery.getActiveProjectWorkspaceRoots(), ["/tmp/workspace"]); + assert.deepEqual(yield* snapshotQuery.getActiveThreadWorktreePaths(), [ + "/tmp/worktree-first", + ]); const project = yield* snapshotQuery.getActiveProjectByWorkspaceRoot("/tmp/workspace"); assert.equal(project._tag, "Some"); diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts index b45e0640bb1..e95f0dab7ce 100644 --- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts +++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts @@ -111,6 +111,9 @@ const ProjectionCountsRowSchema = Schema.Struct({ const ProjectionWorkspaceRootRowSchema = Schema.Struct({ workspaceRoot: Schema.String, }); +const ProjectionWorktreePathRowSchema = Schema.Struct({ + worktreePath: Schema.String, +}); const WorkspaceRootLookupInput = Schema.Struct({ workspaceRoot: Schema.String, }); @@ -331,6 +334,22 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () { `, }); + const listActiveThreadWorktreePathRows = SqlSchema.findAll({ + Request: Schema.Void, + Result: ProjectionWorktreePathRowSchema, + execute: () => + sql` + SELECT DISTINCT threads.worktree_path AS "worktreePath" + FROM projection_threads AS threads + INNER JOIN projection_projects AS projects + ON projects.project_id = threads.project_id + WHERE threads.worktree_path IS NOT NULL + AND threads.deleted_at IS NULL + AND projects.deleted_at IS NULL + ORDER BY threads.worktree_path ASC + `, + }); + const listThreadRows = SqlSchema.findAll({ Request: Schema.Void, Result: ProjectionThreadDbRowSchema, @@ -1764,6 +1783,18 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () { Effect.map((rows) => rows.map((row) => row.workspaceRoot)), ); + const getActiveThreadWorktreePaths: ProjectionSnapshotQueryShape["getActiveThreadWorktreePaths"] = + () => + listActiveThreadWorktreePathRows(undefined).pipe( + Effect.mapError( + toPersistenceSqlOrDecodeError( + "ProjectionSnapshotQuery.getActiveThreadWorktreePaths:query", + "ProjectionSnapshotQuery.getActiveThreadWorktreePaths:decodeRows", + ), + ), + Effect.map((rows) => rows.map((row) => row.worktreePath)), + ); + const getActiveProjectByWorkspaceRoot: ProjectionSnapshotQueryShape["getActiveProjectByWorkspaceRoot"] = (workspaceRoot) => getActiveProjectRowByWorkspaceRoot({ workspaceRoot }).pipe( @@ -2138,6 +2169,7 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () { getSnapshotSequence, getCounts, getActiveProjectWorkspaceRoots, + getActiveThreadWorktreePaths, getActiveProjectByWorkspaceRoot, getProjectShellById, getFirstActiveThreadIdByProjectId, diff --git a/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts b/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts index 3e0066f8799..2c46527a6fa 100644 --- a/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts +++ b/apps/server/src/orchestration/Services/ProjectionSnapshotQuery.ts @@ -116,6 +116,14 @@ export interface ProjectionSnapshotQueryShape { ProjectionRepositoryError >; + /** + * Read persisted worktree paths for non-deleted threads in active projects. + */ + readonly getActiveThreadWorktreePaths: () => Effect.Effect< + ReadonlyArray, + ProjectionRepositoryError + >; + /** * Read the active project for an exact workspace root match. */ diff --git a/apps/server/src/project/ProjectSetupScriptRunner.test.ts b/apps/server/src/project/ProjectSetupScriptRunner.test.ts index 1331fbe289b..136f3a76a0d 100644 --- a/apps/server/src/project/ProjectSetupScriptRunner.test.ts +++ b/apps/server/src/project/ProjectSetupScriptRunner.test.ts @@ -33,6 +33,7 @@ const makeProjectionSnapshotQueryLayer = (project: OrchestrationProject) => getSnapshotSequence: () => Effect.succeed({ snapshotSequence: 1 }), getCounts: () => Effect.die("unused"), getActiveProjectWorkspaceRoots: () => Effect.die("unused"), + getActiveThreadWorktreePaths: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: (workspaceRoot) => Effect.succeed( workspaceRoot === project.workspaceRoot ? Option.some(project) : Option.none(), diff --git a/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts b/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts index c4d585db265..0ae351bd575 100644 --- a/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts +++ b/apps/server/src/provider/Layers/ProviderSessionReaper.test.ts @@ -200,6 +200,7 @@ describe("ProviderSessionReaper", () => { Effect.succeed({ snapshotSequence: input.readModel.snapshotSequence }), getCounts: () => Effect.die("unused"), getActiveProjectWorkspaceRoots: () => Effect.die("unused"), + getActiveThreadWorktreePaths: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.die("unused"), getProjectShellById: () => Effect.die("unused"), getFirstActiveThreadIdByProjectId: () => Effect.die("unused"), diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index 0cc9c79b7d4..8c2f5481d1c 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -122,6 +122,42 @@ describe("ReviewService", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + it.effect("allows an active worktree created with a previous path template", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-workspace-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const repositoryRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-repo-" }); + const legacyWorktreeRoot = path.join(baseDir, "legacy-layout", "feature-local"); + const nestedCwd = path.join(legacyWorktreeRoot, "packages", "app"); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* fs.makeDirectory(nestedCwd, { recursive: true }); + + const result = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review.getDiffPreview({ + cwd: nestedCwd, + repositoryRoots: [repositoryRoot], + knownWorktreePaths: [legacyWorktreeRoot], + }); + }).pipe( + Effect.provide( + makeLayer({ + workspaceRoot, + baseDir, + detectCalls, + worktreePathTemplate: "{repoRoot}/.worktrees/{branch}", + }), + ), + ); + + assert.strictEqual(result.cwd, nestedCwd); + assert.deepStrictEqual(result.sources, []); + assert.deepStrictEqual(detectCalls, [{ cwd: nestedCwd }]); + }).pipe(Effect.provide(NodeServices.layer)), + ); + it.effect("preserves lexical repo names when matching canonical worktree paths", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index ec49495b12f..75814395f4b 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -24,7 +24,10 @@ export class ReviewService extends Context.Service< ReviewService, { readonly getDiffPreview: ( - input: ReviewDiffPreviewInput & { readonly repositoryRoots?: ReadonlyArray }, + input: ReviewDiffPreviewInput & { + readonly repositoryRoots?: ReadonlyArray; + readonly knownWorktreePaths?: ReadonlyArray; + }, ) => Effect.Effect; } >()("t3/review/ReviewService") {} @@ -62,7 +65,10 @@ export const make = Effect.gen(function* () { }; const assertWorkspaceBoundCwd = Effect.fn("ReviewService.assertWorkspaceBoundCwd")(function* ( - input: ReviewDiffPreviewInput & { readonly repositoryRoots?: ReadonlyArray }, + input: ReviewDiffPreviewInput & { + readonly repositoryRoots?: ReadonlyArray; + readonly knownWorktreePaths?: ReadonlyArray; + }, ) { const { cwd } = input; const [candidate, workspaceRoot, worktreesRoot] = yield* Effect.all([ @@ -93,6 +99,22 @@ export const make = Effect.gen(function* () { ), { concurrency: "unbounded" }, ); + const knownWorktreePaths = yield* Effect.forEach( + input.knownWorktreePaths ?? [], + (worktreePath) => + canonicalizePath(worktreePath).pipe( + Effect.catch((cause) => + Effect.logWarning("Skipping worktree path that could not be resolved", { + cause, + worktreePath, + }).pipe(Effect.as(null)), + ), + ), + { concurrency: "unbounded" }, + ); + const matchesKnownWorktreePath = knownWorktreePaths.some( + (worktreePath) => worktreePath !== null && isWithinRoot(candidate, worktreePath), + ); const matchesConfiguredWorktreePath = repositoryRoots.some((repositoryRoot) => repositoryRoot === null ? false @@ -108,6 +130,7 @@ export const make = Effect.gen(function* () { if ( isWithinRoot(candidate, workspaceRoot) || isWithinRoot(candidate, worktreesRoot) || + matchesKnownWorktreePath || matchesConfiguredWorktreePath ) { return; diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 7da35cdc849..93d318a4969 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -717,6 +717,7 @@ const buildAppUnderTest = (options?: { getThreadDetailSnapshot: () => Effect.succeed(Option.none()), getCounts: () => Effect.succeed({ projectCount: 0, threadCount: 0 }), getActiveProjectWorkspaceRoots: () => Effect.succeed([]), + getActiveThreadWorktreePaths: () => Effect.succeed([]), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), getThreadCheckpointContext: () => Effect.succeed(Option.none()), @@ -5242,20 +5243,24 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); - it.effect("loads review project roots without hydrating the shell snapshot", () => + it.effect("loads review project paths without hydrating the shell snapshot", () => Effect.gen(function* () { const repositoryRoots = ["/tmp/project-a", "/tmp/project-b"]; + const knownWorktreePaths = ["/tmp/legacy-layout/feature"]; let receivedRepositoryRoots: ReadonlyArray | undefined; + let receivedKnownWorktreePaths: ReadonlyArray | undefined; yield* buildAppUnderTest({ layers: { projectionSnapshotQuery: { getShellSnapshot: () => Effect.die("review preview must not hydrate shell snapshots"), getActiveProjectWorkspaceRoots: () => Effect.succeed(repositoryRoots), + getActiveThreadWorktreePaths: () => Effect.succeed(knownWorktreePaths), }, reviewService: { getDiffPreview: (input) => Effect.sync(() => { receivedRepositoryRoots = input.repositoryRoots; + receivedKnownWorktreePaths = input.knownWorktreePaths; return { cwd: input.cwd, generatedAt: TEST_EPOCH, @@ -5276,6 +5281,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { ); assert.deepEqual(receivedRepositoryRoots, repositoryRoots); + assert.deepEqual(receivedKnownWorktreePaths, knownWorktreePaths); assert.equal(result.cwd, "/tmp/project-a/.worktrees/feature"); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); diff --git a/apps/server/src/serverRuntimeStartup.test.ts b/apps/server/src/serverRuntimeStartup.test.ts index 3ddea43313a..520567829f0 100644 --- a/apps/server/src/serverRuntimeStartup.test.ts +++ b/apps/server/src/serverRuntimeStartup.test.ts @@ -90,6 +90,7 @@ it.effect("launchStartupHeartbeat does not block the caller while counts are loa }), ), getActiveProjectWorkspaceRoots: () => Effect.die("unused"), + getActiveThreadWorktreePaths: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.succeed(Option.none()), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -142,6 +143,7 @@ it.effect("resolveAutoBootstrapWelcomeTargets returns existing project and threa getSnapshotSequence: () => Effect.die("unused"), getCounts: () => Effect.die("unused"), getActiveProjectWorkspaceRoots: () => Effect.die("unused"), + getActiveThreadWorktreePaths: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed( Option.some({ @@ -199,6 +201,7 @@ it.effect("resolveAutoBootstrapWelcomeTargets creates a project and thread when getSnapshotSequence: () => Effect.die("unused"), getCounts: () => Effect.die("unused"), getActiveProjectWorkspaceRoots: () => Effect.die("unused"), + getActiveThreadWorktreePaths: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.die("unused"), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), @@ -250,6 +253,7 @@ it.effect("resolveAutoBootstrapWelcomeTargets preserves typed UUID generation fa getSnapshotSequence: () => Effect.die("unused"), getCounts: () => Effect.die("unused"), getActiveProjectWorkspaceRoots: () => Effect.die("unused"), + getActiveThreadWorktreePaths: () => Effect.die("unused"), getActiveProjectByWorkspaceRoot: () => Effect.succeed(Option.none()), getProjectShellById: () => Effect.die("unused"), getFirstActiveThreadIdByProjectId: () => Effect.succeed(Option.none()), diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 1f2903f70b2..89d62f5f85e 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -134,6 +134,31 @@ it.effect("expands path template placeholders in a single pass", () => }).pipe(Effect.provide(NodeServices.layer)), ); +it.effect("treats branch marker text in path segments literally", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const cwd = path.resolve("repos", "example"); + const worktreesDir = path.resolve("central-worktrees", "__t3_worktree_branch__"); + + assert.isTrue( + matchesWorktreePathTemplate(path, { + candidate: path.join(worktreesDir, "feature"), + cwd, + worktreesDir, + template: "{worktreesDir}/{branch}", + }), + ); + assert.isFalse( + matchesWorktreePathTemplate(path, { + candidate: path.resolve("central-worktrees", "different", "feature"), + cwd, + worktreesDir, + template: "{worktreesDir}/{branch}", + }), + ); + }).pipe(Effect.provide(NodeServices.layer)), +); + it.effect("uses stable diagnostics for every parsed non-repository command", () => { const commands: Array<{ readonly args: ReadonlyArray; readonly lcAll?: string }> = []; const spawner = ChildProcessSpawner.make((command) => diff --git a/apps/server/src/vcs/worktreePathTemplate.ts b/apps/server/src/vcs/worktreePathTemplate.ts index 1d914387628..de3ffca1685 100644 --- a/apps/server/src/vcs/worktreePathTemplate.ts +++ b/apps/server/src/vcs/worktreePathTemplate.ts @@ -43,7 +43,17 @@ export function matchesWorktreePathTemplate( path: Path.Path, input: Omit & { readonly candidate: string }, ): boolean { - const branchMarker = "__t3_worktree_branch__"; + let branchMarker = "__t3_worktree_branch__"; + const markerInputs = [ + input.cwd, + input.resolvedRepoRoot ?? "", + input.worktreesDir, + input.template, + input.candidate, + ]; + while (markerInputs.some((value) => value.includes(branchMarker))) { + branchMarker += "_"; + } const resolvedTemplate = resolveWorktreePathTemplate(path, { ...input, branch: branchMarker, diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index c4a8a15eb1f..754568b94ee 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1887,21 +1887,26 @@ const makeWsRpcLayer = ( observeRpcEffect( WS_METHODS.reviewGetDiffPreview, Effect.gen(function* () { - const repositoryRoots = yield* projectionSnapshotQuery - .getActiveProjectWorkspaceRoots() - .pipe( - Effect.mapError( - (cause) => - new VcsRepositoryDetectionError({ - operation: "review.getDiffPreview", - cwd: input.cwd, - detail: - "Failed to load project roots required to validate the review workspace.", - cause, - }), - ), - ); - return yield* review.getDiffPreview({ ...input, repositoryRoots }); + const [repositoryRoots, knownWorktreePaths] = yield* Effect.all([ + projectionSnapshotQuery.getActiveProjectWorkspaceRoots(), + projectionSnapshotQuery.getActiveThreadWorktreePaths(), + ]).pipe( + Effect.mapError( + (cause) => + new VcsRepositoryDetectionError({ + operation: "review.getDiffPreview", + cwd: input.cwd, + detail: + "Failed to load project paths required to validate the review workspace.", + cause, + }), + ), + ); + return yield* review.getDiffPreview({ + ...input, + repositoryRoots, + knownWorktreePaths, + }); }), { "rpc.aggregate": "review", From b8aa507bc229db74761453f71d01920fd4d9f6f0 Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Mon, 27 Jul 2026 19:23:54 -0700 Subject: [PATCH 9/9] test(server): match review path load error --- apps/server/src/server.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 93d318a4969..939a8eac8fc 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -5327,7 +5327,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { const detectionError = result.failure; assert.equal(detectionError.operation, "review.getDiffPreview"); assert.equal(detectionError.cwd, "/tmp/repository-worktrees/feature"); - assert.include(detectionError.detail, "Failed to load project roots"); + assert.include(detectionError.detail, "Failed to load project paths"); assert.instanceOf(detectionError.cause, Error); assert.include(detectionError.cause.message, projectionError.message); assert.equal(reviewCalls, 0);