From 4a1eef37e3e7d9c87595dc4a24fc62fdb3700da4 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sat, 12 Sep 2026 13:04:25 +0200 Subject: [PATCH 01/71] fix(web): resize review file trees and show active mode --- apps/web/src/components/DiffPanel.tsx | 18 +++--- .../components/diffs/DiffFileTree.test.tsx | 47 ++++++++++++++- .../web/src/components/diffs/DiffFileTree.tsx | 42 ++++++++++++- .../pullRequest/PullRequestCodeTab.tsx | 59 ++++++++++--------- 4 files changed, 124 insertions(+), 42 deletions(-) diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index d764b9c6a9e2..f097cf616c19 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -862,6 +862,7 @@ export default function DiffPanel({ render={ {fileTreeOpen ? ( - + ) : null} ) : ( diff --git a/apps/web/src/components/diffs/DiffFileTree.test.tsx b/apps/web/src/components/diffs/DiffFileTree.test.tsx index 3a97c60254ab..5b6f303a004b 100644 --- a/apps/web/src/components/diffs/DiffFileTree.test.tsx +++ b/apps/web/src/components/diffs/DiffFileTree.test.tsx @@ -1,12 +1,13 @@ import type { CodeViewScrollTarget } from "@pierre/diffs"; import type { FileTree as FileTreeModel } from "@pierre/trees"; import { FileTree } from "@pierre/trees/react"; -import { act, type MouseEvent, type ReactNode } from "react"; +import { act, type MouseEvent, type PointerEvent, type ReactNode } from "react"; import { create, type ReactTestRenderer } from "react-test-renderer"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { DiffFileTree, type DiffFileTreeEntry } from "./DiffFileTree"; import { useCodeViewFileReveal } from "./useCodeViewFileReveal"; +import { RightPanelResizeHandle } from "../preview/RightPanelResizeHandle"; vi.mock("../../hooks/useTheme", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) })); // Tooltip positioning is unrelated to the tree's actual model and activation path. @@ -41,13 +42,16 @@ describe("diff tree file activation", () => { function Panel({ files = entries, selectedPath = null, + widthStorageKey = "t3code.diffFileTreeWidth", }: { files?: DiffFileTreeEntry[]; selectedPath?: string | null; + widthStorageKey?: string; }) { const reveal = useCodeViewFileReveal(viewer, "working-tree"); return ( { vi.useFakeTimers(); vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); vi.stubGlobal("HTMLElement", TreeRow); + vi.stubGlobal("window", new EventTarget()); }); afterEach(async () => { @@ -112,6 +117,46 @@ describe("diff tree file activation", () => { vi.unstubAllGlobals(); }); + it("stores resized widths independently for each review surface", async () => { + const widths = new Map(); + Object.assign(window, { + localStorage: { + getItem: (key: string) => widths.get(key) ?? null, + setItem: (key: string, value: string) => widths.set(key, value), + }, + }); + vi.stubGlobal("document", { body: { style: { removeProperty() {} } } }); + vi.stubGlobal("requestAnimationFrame", () => 1); + vi.stubGlobal("cancelAnimationFrame", () => {}); + const pointer = { + button: 0, + pointerId: 1, + clientX: 100, + preventDefault() {}, + stopPropagation() {}, + currentTarget: { + setPointerCapture() {}, + hasPointerCapture: () => true, + releasePointerCapture() {}, + }, + } as unknown as PointerEvent; + for (const widthStorageKey of ["t3code.diffFileTreeWidth", "t3code.pullRequestFileTreeWidth"]) { + await mount({ widthStorageKey }); + const { handlers } = renderer!.root.findByType(RightPanelResizeHandle).props; + await act(async () => { + handlers.onPointerDown(pointer); + handlers.onPointerMove({ ...pointer, clientX: 60 }); + handlers.onPointerUp(pointer); + }); + await act(async () => renderer!.unmount()); + renderer = undefined; + } + expect([...widths]).toEqual([ + ["t3code.diffFileTreeWidth", "296"], + ["t3code.pullRequestFileTreeWidth", "296"], + ]); + }); + it("reissues the reveal when the sole selected file is activated again", async () => { await mount(); await activate("02-short.ts"); diff --git a/apps/web/src/components/diffs/DiffFileTree.tsx b/apps/web/src/components/diffs/DiffFileTree.tsx index 0d200853bcb4..65a06ba40326 100644 --- a/apps/web/src/components/diffs/DiffFileTree.tsx +++ b/apps/web/src/components/diffs/DiffFileTree.tsx @@ -1,15 +1,17 @@ import type { GitStatusEntry } from "@pierre/trees"; import { FileTree, useFileTree, useFileTreeSelector } from "@pierre/trees/react"; import { ChevronsDownUpIcon, ChevronsUpDownIcon } from "lucide-react"; -import { useEffect, useMemo, useRef, type ReactNode } from "react"; +import { useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from "react"; import { useTheme } from "~/hooks/useTheme"; +import { useResizableWidth } from "~/hooks/useResizableWidth"; import { cn } from "~/lib/utils"; import { T3_PIERRE_ICONS } from "~/pierre-icons"; import { PIERRE_TREE_UNSAFE_CSS, pierreTreeStyle } from "~/pierre-tree-theme"; import { areAllDirectoriesExpanded, setAllDirectoriesExpanded } from "../files/fileTreeExpansion"; import { Button } from "../ui/button"; +import { RightPanelResizeHandle } from "../preview/RightPanelResizeHandle"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { buildDiffFileTreeUpdates, @@ -30,11 +32,13 @@ interface DiffFileTreeProps { readonly selectedPath?: string | null; readonly revealRequestId?: number; readonly ariaLabel: string; + readonly widthStorageKey: string; /** Right-aligned content in the header row, after the file count. */ readonly headerAccessory?: ReactNode; /** Rendered under the tree, for a host that still has files to fetch. */ readonly footer?: ReactNode; readonly className?: string; + readonly defaultWidth?: number; } /** @@ -47,11 +51,31 @@ export function DiffFileTree({ selectedPath = null, revealRequestId = 0, ariaLabel, + widthStorageKey, headerAccessory, footer, className, + defaultWidth = 256, }: DiffFileTreeProps) { const { resolvedTheme } = useTheme(); + const containerRef = useRef(null); + const [maxWidth, setMaxWidth] = useState(640); + useLayoutEffect(() => { + const parent = containerRef.current?.parentElement; + if (!parent) return; + const resize = () => setMaxWidth(Math.max(160, Math.min(640, parent.clientWidth * 0.6))); + resize(); + const observer = new ResizeObserver(resize); + observer.observe(parent); + return () => observer.disconnect(); + }, []); + const { width, handlers } = useResizableWidth({ + storageKey: widthStorageKey, + defaultWidth, + minWidth: 160, + maxWidth, + edge: "left", + }); const paths = useMemo(() => entries.map((entry) => entry.path), [entries]); const directoryPaths = useMemo(() => collectDirectoryPaths(paths), [paths]); const gitStatus = useMemo>( @@ -83,7 +107,11 @@ export function DiffFileTree({ }, paths: [], search: false, - unsafeCSS: PIERRE_TREE_UNSAFE_CSS, + unsafeCSS: `${PIERRE_TREE_UNSAFE_CSS} + [data-file-tree-virtualized-scroll='true'] { + overflow-x: hidden; + } + `, }); const allDirectoriesExpanded = useFileTreeSelector(model, (currentModel) => areAllDirectoriesExpanded(currentModel, directoryPaths), @@ -137,7 +165,15 @@ export function DiffFileTree({ }, [model, paths, revealRequestId, selectedPath]); return ( -
+
+
{fileTreeOpen ? ( -
- ) - } - /> - + + +
+ ) + } + /> ) : null}
{unstructured} From e2403c143c300b37fea309706fa064cc314d41f3 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:35:05 +0200 Subject: [PATCH 02/71] fix(web): use one control to fold review files and folders --- apps/web/src/components/DiffPanel.tsx | 7 ++- .../components/diffs/DiffFileTree.test.tsx | 20 +++++- .../web/src/components/diffs/DiffFileTree.tsx | 61 ++++++++----------- .../pullRequest/PullRequestCodeTab.tsx | 5 +- 4 files changed, 52 insertions(+), 41 deletions(-) diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index f097cf616c19..61b92ba266d6 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -54,7 +54,7 @@ import { DiffFilePathCopyButton } from "./DiffFilePathCopyButton"; import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell"; import { DiffStatLabel } from "./chat/DiffStatLabel"; import { AnnotatableCodeView, type AnnotatableCodeViewHandle } from "./diffs/AnnotatableCodeView"; -import { DiffFileTree } from "./diffs/DiffFileTree"; +import { DiffFileTree, type DiffFileTreeHandle } from "./diffs/DiffFileTree"; import { diffFileTreeEntries } from "./diffs/diffFileTree.logic"; import { Button } from "./ui/button"; import { ToggleGroup, Toggle } from "./ui/toggle-group"; @@ -515,7 +515,9 @@ export default function DiffPanel({ [collapseScopeKey], ); + const treeRef = useRef(null); const toggleDiffFileCollapse = useCallback(() => { + treeRef.current?.setExpanded(allDiffFilesCollapsed); setCodeViewRevision((current) => current + 1); setCollapsedDiffFiles((current) => { const currentKeys = @@ -526,7 +528,7 @@ export default function DiffPanel({ fileKeys: toggleAllDiffFiles(diffFileKeys, currentKeys), }; }); - }, [collapseScopeKey, diffFileKeys]); + }, [allDiffFilesCollapsed, collapseScopeKey, diffFileKeys]); const selectTurn = (turnId: TurnId) => { if (!routeThreadRef) return; @@ -1029,6 +1031,7 @@ export default function DiffPanel({ {fileTreeOpen ? ( { let renderer: ReactTestRenderer | undefined; const targets: CodeViewScrollTarget[] = []; + const treeRef = createRef(); const viewer = { getInstance: () => viewer, scrollTo: (target: CodeViewScrollTarget) => targets.push(target), @@ -51,6 +52,7 @@ describe("diff tree file activation", () => { const reveal = useCodeViewFileReveal(viewer, "working-tree"); return ( { expect(targets).toEqual([]); }); + it("lets the panel control all folders without a second collapse button", async () => { + await mount({ files: [{ path: "src/nested/app.ts", status: "modified" }] }); + await act(async () => treeRef.current!.setExpanded(false)); + for (const path of ["src/", "src/nested/"]) { + const directory = model().getItem(path)!; + expect("isExpanded" in directory && directory.isExpanded()).toBe(false); + } + await act(async () => treeRef.current!.setExpanded(true)); + for (const path of ["src/", "src/nested/"]) { + const directory = model().getItem(path)!; + expect("isExpanded" in directory && directory.isExpanded()).toBe(true); + } + }); + it("does not echo controlled selection, but lets the reader activate it", async () => { await mount({ selectedPath: "02-short.ts" }); expect(model().getSelectedPaths()).toEqual(["02-short.ts"]); diff --git a/apps/web/src/components/diffs/DiffFileTree.tsx b/apps/web/src/components/diffs/DiffFileTree.tsx index 65a06ba40326..43c7b482b1b0 100644 --- a/apps/web/src/components/diffs/DiffFileTree.tsx +++ b/apps/web/src/components/diffs/DiffFileTree.tsx @@ -1,7 +1,15 @@ import type { GitStatusEntry } from "@pierre/trees"; -import { FileTree, useFileTree, useFileTreeSelector } from "@pierre/trees/react"; -import { ChevronsDownUpIcon, ChevronsUpDownIcon } from "lucide-react"; -import { useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from "react"; +import { FileTree, useFileTree } from "@pierre/trees/react"; +import { + useEffect, + useImperativeHandle, + useLayoutEffect, + useMemo, + useRef, + useState, + type ReactNode, + type Ref, +} from "react"; import { useTheme } from "~/hooks/useTheme"; import { useResizableWidth } from "~/hooks/useResizableWidth"; @@ -9,10 +17,8 @@ import { cn } from "~/lib/utils"; import { T3_PIERRE_ICONS } from "~/pierre-icons"; import { PIERRE_TREE_UNSAFE_CSS, pierreTreeStyle } from "~/pierre-tree-theme"; -import { areAllDirectoriesExpanded, setAllDirectoriesExpanded } from "../files/fileTreeExpansion"; -import { Button } from "../ui/button"; +import { setAllDirectoriesExpanded } from "../files/fileTreeExpansion"; import { RightPanelResizeHandle } from "../preview/RightPanelResizeHandle"; -import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { buildDiffFileTreeUpdates, collectDirectoryPaths, @@ -21,7 +27,12 @@ import { export type { DiffFileTreeEntry } from "./diffFileTree.logic"; +export interface DiffFileTreeHandle { + setExpanded: (expanded: boolean) => void; +} + interface DiffFileTreeProps { + readonly ref?: Ref; readonly entries: ReadonlyArray; /** Called with the file's path when the reader picks a file row. */ readonly onSelectFile: (path: string) => void; @@ -46,6 +57,7 @@ interface DiffFileTreeProps { * compared to a workspace, and the reader came for the files, not the folders. */ export function DiffFileTree({ + ref, entries, onSelectFile, selectedPath = null, @@ -113,8 +125,13 @@ export function DiffFileTree({ } `, }); - const allDirectoriesExpanded = useFileTreeSelector(model, (currentModel) => - areAllDirectoriesExpanded(currentModel, directoryPaths), + useImperativeHandle( + ref, + () => ({ + setExpanded: (expanded: boolean) => + setAllDirectoriesExpanded(model, directoryPaths, expanded), + }), + [model, directoryPaths], ); useEffect(() => { @@ -181,34 +198,6 @@ export function DiffFileTree({ Files {entries.length} {headerAccessory} - {directoryPaths.length > 0 ? ( - - - setAllDirectoriesExpanded(model, directoryPaths, !allDirectoriesExpanded) - } - /> - } - > - {allDirectoriesExpanded ? ( - - ) : ( - - )} - - - {allDirectoriesExpanded ? "Collapse all folders" : "Expand all folders"} - - - ) : null} (null); const toggleAllFiles = () => { + treeRef.current?.setExpanded(allFilesCollapsed); // Held as an override of the default rather than as the file keys on screen: a diff that is // still paging would otherwise bring its next slice in folded, moments after the reader // asked for everything to be open. @@ -1406,6 +1408,7 @@ function PullRequestCodeTab({ {fileTreeOpen ? ( Date: Sat, 12 Sep 2026 13:07:12 +0200 Subject: [PATCH 03/71] fix(server): guard review saves against checkout changes --- .../server/src/git/GitWorkflowService.test.ts | 126 ++++++++++++++++++ apps/server/src/git/GitWorkflowService.ts | 55 +++++++- apps/server/src/server.test.ts | 64 ++++++++- apps/server/src/ws.ts | 62 +++++++-- packages/contracts/src/project.ts | 3 + 5 files changed, 297 insertions(+), 13 deletions(-) diff --git a/apps/server/src/git/GitWorkflowService.test.ts b/apps/server/src/git/GitWorkflowService.test.ts index 2ea14b951fe2..23d550a055e2 100644 --- a/apps/server/src/git/GitWorkflowService.test.ts +++ b/apps/server/src/git/GitWorkflowService.test.ts @@ -1,6 +1,12 @@ import { assert, describe, expect, it, vi } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as Deferred from "effect/Deferred"; +import * as Fiber from "effect/Fiber"; +import * as DateTime from "effect/DateTime"; +import * as Option from "effect/Option"; import * as Layer from "effect/Layer"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; import { VcsRepositoryDetectionError } from "@t3tools/contracts"; @@ -8,11 +14,14 @@ import * as GitManager from "./GitManager.ts"; import * as GitWorkflowService from "./GitWorkflowService.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; +import * as VcsDriver from "../vcs/VcsDriver.ts"; function makeLayer(input: { readonly detect: VcsDriverRegistry.VcsDriverRegistry["Service"]["detect"]; }) { return GitWorkflowService.layer.pipe( + Layer.provide(Path.layer), + Layer.provide(FileSystem.layerNoop({ realPath: (path) => Effect.succeed(path) })), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ detect: input.detect, @@ -24,6 +33,121 @@ function makeLayer(input: { } describe("GitWorkflowService", () => { + it.effect.each(["checkout", "pull", "refresh"])("serializes %s with writes", (action) => + Effect.gen(function* () { + const entered = yield* Deferred.make(); + const release = yield* Deferred.make(); + const commandResolved = yield* Deferred.make(); + const events: string[] = []; + const observedAt = yield* DateTime.now; + const driver = yield* VcsDriver.VcsDriver.pipe( + Effect.provide( + Layer.mock(VcsDriver.VcsDriver)({ + capabilities: { + kind: "git", + supportsWorktrees: true, + supportsBookmarks: false, + supportsAtomicSnapshot: false, + supportsPushDefaultRemote: true, + ignoreClassifier: "native", + }, + }), + ), + ); + const workflow = yield* GitWorkflowService.make.pipe( + Effect.provide( + Layer.mergeAll( + Path.layer, + FileSystem.layerNoop({ realPath: (path) => Effect.succeed(path) }), + Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ + resolve: ({ cwd }) => + Effect.gen(function* () { + if (cwd === "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/repo/nested") yield* Deferred.succeed(commandResolved, undefined); + return { + kind: "git" as const, + driver, + repository: { + kind: "git" as const, + rootPath: + cwd === "/other" ? "/other" : cwd === "/pr-worktree" ? cwd : "/repo", + metadataPath: + cwd === "/pr-worktree" + ? "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/repo/.git" + : cwd === "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/repo/nested" + ? "../.git" + : ".git", + freshness: { + source: "live-local" as const, + observedAt, + expiresAt: Option.none(), + }, + }, + }; + }), + }), + Layer.mock(GitVcsDriver.GitVcsDriver)({ + switchRef: ({ refName }) => + Effect.sync(() => { + events.push("checkout"); + return { refName }; + }), + pullCurrentBranch: () => + Effect.sync(() => { + events.push("pull"); + return { + status: "pulled" as const, + refName: "main", + upstreamRef: "origin/main", + }; + }), + }), + Layer.mock(GitManager.GitManager)({ + preparePullRequestThread: () => + Effect.sync(() => { + events.push("refresh"); + return { worktreePath: "/pr-worktree" } as never; + }), + }), + ), + ), + ); + const write = yield* workflow + .withRepositoryLock( + action === "refresh" ? "/pr-worktree" : "/repo", + Effect.gen(function* () { + events.push("validate"); + yield* Deferred.succeed(entered, undefined); + yield* Deferred.await(release); + events.push("write"); + }), + ) + .pipe(Effect.forkScoped); + yield* Deferred.await(entered); + const command = yield* ( + action === "checkout" + ? workflow.switchRef({ cwd: "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/repo/nested", refName: "other" }) + : action === "pull" + ? workflow.pullCurrentBranch("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/repo/nested") + : workflow.preparePullRequestThread({ + cwd: "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/repo/nested", + reference: "42", + mode: "worktree", + }) + ).pipe(Effect.forkScoped); + yield* Deferred.await(commandResolved); + yield* Effect.yieldNow; + yield* workflow.withRepositoryLock( + "/other", + Effect.sync(() => events.push("other repository")), + ); + assert.deepStrictEqual(events, ["validate", "other repository"]); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(write); + yield* Fiber.join(command); + assert.deepStrictEqual(events, ["validate", "other repository", "write", action]); + }).pipe(Effect.scoped), + ); + it.effect("returns an empty local status when no VCS repository is detected", () => Effect.gen(function* () { const workflow = yield* GitWorkflowService.GitWorkflowService; @@ -87,6 +211,8 @@ describe("GitWorkflowService", () => { const status = vi.fn(); const testLayer = GitWorkflowService.layer.pipe( + Layer.provide(Path.layer), + Layer.provide(FileSystem.layerNoop({ realPath: (path) => Effect.succeed(path) })), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ detect: () => Effect.succeed(null), diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index e90a59fae8f1..97a5a4319cba 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -1,6 +1,9 @@ import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; +import * as Semaphore from "effect/Semaphore"; import { GitManagerError, @@ -35,6 +38,10 @@ import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; export class GitWorkflowService extends Context.Service< GitWorkflowService, { + readonly withRepositoryLock: ( + cwd: string, + effect: Effect.Effect, + ) => Effect.Effect; readonly status: ( input: VcsStatusInput, ) => Effect.Effect; @@ -145,8 +152,38 @@ function nonRepositoryListRefs(): VcsListRefsResult { /** @public Service construction is part of the canonical Effect module API. */ export const make = Effect.gen(function* () { const registry = yield* VcsDriverRegistry.VcsDriverRegistry; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; const git = yield* GitVcsDriver.GitVcsDriver; const gitManager = yield* GitManager.GitManager; + const repositoryLocks = new Map(); + const withRepositoryLock: GitWorkflowService["Service"]["withRepositoryLock"] = (cwd, effect) => + Effect.gen(function* () { + const key = yield* Effect.gen(function* () { + const handle = yield* registry.resolve({ cwd }); + if (handle.repository.metadataPath === null) { + return yield* Effect.fail("The repository metadata path is unavailable."); + } + return yield* fileSystem.realPath(path.resolve(cwd, handle.repository.metadataPath)); + }).pipe( + Effect.mapError( + (cause) => + new GitCommandError({ + operation: "GitWorkflowService.withRepositoryLock", + command: "vcs-route", + cwd, + detail: "Failed to resolve the repository before changing its files.", + cause, + }), + ), + ); + let lock = repositoryLocks.get(key); + if (!lock) { + lock = Semaphore.makeUnsafe(1); + repositoryLocks.set(key, lock); + } + return yield* lock.withPermits(1)(effect); + }); const ensureGit = Effect.fn("GitWorkflowService.ensureGit")(function* ( operation: string, @@ -263,6 +300,7 @@ export const make = Effect.gen(function* () { ensureGit(operation, input.cwd).pipe(Effect.andThen(run(input))); return GitWorkflowService.of({ + withRepositoryLock, status: (input) => detectGitRepositoryForStatus("GitWorkflowService.status", input.cwd).pipe( Effect.flatMap((isGitRepository) => @@ -289,19 +327,25 @@ export const make = Effect.gen(function* () { pullCurrentBranch: (cwd) => ensureGitCommand("GitWorkflowService.pullCurrentBranch", cwd).pipe( Effect.andThen(git.pullCurrentBranch(cwd)), + (effect) => withRepositoryLock(cwd, effect), ), runStackedAction: (input, options) => ensureGit("GitWorkflowService.runStackedAction", input.cwd).pipe( Effect.andThen(gitManager.runStackedAction(input, options)), + (effect) => (input.featureBranch ? withRepositoryLock(input.cwd, effect) : effect), ), resolvePullRequest: routeGitManager( "GitWorkflowService.resolvePullRequest", gitManager.resolvePullRequest, ), - preparePullRequestThread: routeGitManager( - "GitWorkflowService.preparePullRequestThread", - gitManager.preparePullRequestThread, - ), + preparePullRequestThread: (input) => + withRepositoryLock( + input.cwd, + routeGitManager( + "GitWorkflowService.preparePullRequestThread", + gitManager.preparePullRequestThread, + )(input), + ), listRefs: (input) => detectGitRepositoryForCommand("GitWorkflowService.listRefs", input.cwd).pipe( Effect.flatMap((isGitRepository) => @@ -339,14 +383,17 @@ export const make = Effect.gen(function* () { createRef: (input) => ensureGitCommand("GitWorkflowService.createRef", input.cwd).pipe( Effect.andThen(git.createRef(input)), + (effect) => (input.switchRef ? withRepositoryLock(input.cwd, effect) : effect), ), switchRef: (input) => ensureGitCommand("GitWorkflowService.switchRef", input.cwd).pipe( Effect.andThen(Effect.scoped(git.switchRef(input))), + (effect) => withRepositoryLock(input.cwd, effect), ), renameBranch: (input) => ensureGit("GitWorkflowService.renameBranch", input.cwd).pipe( Effect.andThen(git.renameBranch(input)), + (effect) => withRepositoryLock(input.cwd, effect), ), }); }); diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 81399ce61d6f..1bb027c37d1d 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -671,7 +671,7 @@ const buildAppUnderTest = (options?: { kind: input.requestedKind === "auto" || !input.requestedKind ? "git" : input.requestedKind, rootPath: input.cwd, - metadataPath: null, + metadataPath: tempBaseDir, freshness: { source: "live-local", observedAt: TEST_EPOCH, @@ -7085,6 +7085,68 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + it.effect("rejects a delayed workspace save after the checkout changes", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-ws-guarded-write-" }); + let branch: string | null = "review"; + let invalidated = false; + yield* buildAppUnderTest({ + layers: { + vcsDriver: { isInsideWorkTree: () => Effect.succeed(true) }, + gitManager: { + invalidateLocalStatus: () => + Effect.sync(() => { + invalidated = true; + }), + localStatus: () => + Effect.sync(() => { + assert.isTrue(invalidated); + invalidated = false; + return { + isRepo: true, + hasPrimaryRemote: false, + isDefaultRef: false, + refName: branch, + hasWorkingTreeChanges: false, + workingTree: { files: [], insertions: 0, deletions: 0 }, + }; + }), + }, + }, + }); + const wsUrl = yield* getWsServerUrl("/ws"); + yield* Effect.scoped( + withWsRpcClient(wsUrl, (client) => + Effect.gen(function* () { + const input = { + cwd, + relativePath: "file.ts", + contents: "saved", + expectedBranch: "review", + }; + yield* client[WS_METHODS.projectsWriteFile](input); + assert.isTrue(invalidated); + for (const nextBranch of ["other", null]) { + branch = nextBranch; + const result = yield* client[WS_METHODS.projectsWriteFile]({ + ...input, + contents: "pending edit", + }).pipe(Effect.result); + assert.equal(result._tag, "Failure"); + if (result._tag === "Failure") { + assert.include(result.failure.message, "checkout changed"); + assert.propertyVal(result.failure, "failure", "checkout_changed"); + } + assert.equal(yield* fs.readFileString(path.join(cwd, "file.ts")), "saved"); + } + }), + ), + ); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + it.effect("creates a missing workspace root during websocket project.create dispatch", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index e91fadd3155e..0a50b42ae6ee 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -2384,16 +2384,62 @@ const makeWsRpcLayer = ( [WS_METHODS.projectsWriteFile]: (input) => observeRpcEffect( WS_METHODS.projectsWriteFile, - workspaceFileSystem.writeFile(input).pipe( - Effect.mapError( - (cause) => - new ProjectWriteFileError({ + Effect.gen(function* () { + if (input.expectedBranch !== undefined) { + yield* gitWorkflow.invalidateLocalStatus(input.cwd); + const status = yield* gitWorkflow.localStatus({ cwd: input.cwd }).pipe( + Effect.mapError( + (cause) => + new ProjectWriteFileError({ + cwd: input.cwd, + relativePath: input.relativePath, + message: "Could not verify the checkout before saving.", + failure: "operation_failed", + cause, + }), + ), + ); + if (!status.isRepo || status.refName !== input.expectedBranch) { + return yield* new ProjectWriteFileError({ cwd: input.cwd, relativePath: input.relativePath, - ...projectFileFailureContext(cause), - cause, - }), - ), + message: "The checkout changed. Reopen the file before saving.", + failure: "checkout_changed", + }); + } + } + return yield* workspaceFileSystem.writeFile(input).pipe( + Effect.tap(() => + input.expectedBranch !== undefined + ? gitWorkflow.invalidateLocalStatus(input.cwd) + : Effect.void, + ), + Effect.mapError( + (cause) => + new ProjectWriteFileError({ + cwd: input.cwd, + relativePath: input.relativePath, + ...projectFileFailureContext(cause), + cause, + }), + ), + ); + }).pipe((effect) => + input.expectedBranch === undefined + ? effect + : gitWorkflow.withRepositoryLock(input.cwd, effect).pipe( + Effect.mapError((cause) => + cause._tag === "ProjectWriteFileError" + ? cause + : new ProjectWriteFileError({ + cwd: input.cwd, + relativePath: input.relativePath, + failure: "operation_failed", + message: "Could not verify the working copy before saving.", + cause, + }), + ), + ), ), { "rpc.aggregate": "workspace" }, ), diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index bee2e70d5746..b7c971da266d 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -212,6 +212,7 @@ export const ProjectFileFailure = Schema.Literals([ "resolved_path_outside_root", "path_not_file", "binary_file", + "checkout_changed", "operation_failed", ]); export type ProjectFileFailure = typeof ProjectFileFailure.Type; @@ -232,6 +233,7 @@ type ProjectFileFailureContext = { readonly cwd: string; readonly relativePath: string; readonly failure: ProjectFileFailure; + readonly message?: string; readonly resolvedPath?: string; readonly resolvedWorkspaceRoot?: string; readonly operation?: ProjectFileOperation; @@ -268,6 +270,7 @@ export const ProjectWriteFileInput = Schema.Struct({ cwd: TrimmedNonEmptyString, relativePath: TrimmedNonEmptyString.check(Schema.isMaxLength(PROJECT_WRITE_FILE_PATH_MAX_LENGTH)), contents: Schema.String, + expectedBranch: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), }); export type ProjectWriteFileInput = typeof ProjectWriteFileInput.Type; From 80ef58c95cf364a923c985f1add3e1ad0460ed87 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sat, 12 Sep 2026 16:13:22 +0200 Subject: [PATCH 04/71] fix(server): serialize worktree removal with review saves --- .../server/src/git/GitWorkflowService.test.ts | 20 ++++++++++++------- apps/server/src/git/GitWorkflowService.ts | 1 + 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/apps/server/src/git/GitWorkflowService.test.ts b/apps/server/src/git/GitWorkflowService.test.ts index 23d550a055e2..dab77c3d501e 100644 --- a/apps/server/src/git/GitWorkflowService.test.ts +++ b/apps/server/src/git/GitWorkflowService.test.ts @@ -33,7 +33,7 @@ function makeLayer(input: { } describe("GitWorkflowService", () => { - it.effect.each(["checkout", "pull", "refresh"])("serializes %s with writes", (action) => + it.effect.each(["checkout", "pull", "refresh", "remove"])("serializes %s with writes", (action) => Effect.gen(function* () { const entered = yield* Deferred.make(); const release = yield* Deferred.make(); @@ -86,6 +86,10 @@ describe("GitWorkflowService", () => { }), }), Layer.mock(GitVcsDriver.GitVcsDriver)({ + removeWorktree: () => + Effect.sync(() => { + events.push("remove"); + }), switchRef: ({ refName }) => Effect.sync(() => { events.push("checkout"); @@ -113,7 +117,7 @@ describe("GitWorkflowService", () => { ); const write = yield* workflow .withRepositoryLock( - action === "refresh" ? "/pr-worktree" : "/repo", + action === "refresh" || action === "remove" ? "/pr-worktree" : "/repo", Effect.gen(function* () { events.push("validate"); yield* Deferred.succeed(entered, undefined); @@ -128,11 +132,13 @@ describe("GitWorkflowService", () => { ? workflow.switchRef({ cwd: "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/repo/nested", refName: "other" }) : action === "pull" ? workflow.pullCurrentBranch("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/repo/nested") - : workflow.preparePullRequestThread({ - cwd: "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/repo/nested", - reference: "42", - mode: "worktree", - }) + : action === "remove" + ? workflow.removeWorktree({ cwd: "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/repo/nested", path: "/pr-worktree", force: false }) + : workflow.preparePullRequestThread({ + cwd: "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/repo/nested", + reference: "42", + mode: "worktree", + }) ).pipe(Effect.forkScoped); yield* Deferred.await(commandResolved); yield* Effect.yieldNow; diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index 97a5a4319cba..b2bb43043a13 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -375,6 +375,7 @@ export const make = Effect.gen(function* () { removeWorktree: (input) => ensureGitCommand("GitWorkflowService.removeWorktree", input.cwd).pipe( Effect.andThen(git.removeWorktree(input)), + (effect) => withRepositoryLock(input.cwd, effect), ), pruneWorktrees: (input) => ensureGitCommand("GitWorkflowService.pruneWorktrees", input.cwd).pipe( From 7bdd593dce2997ad71eccdf1e081c0477ed61da5 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sat, 12 Sep 2026 13:29:30 +0200 Subject: [PATCH 05/71] feat(web): edit review files with explicit save and unsaved warnings --- apps/web/src/components/DiffPanel.tsx | 47 ++- .../diffs/DiffFileEditButton.test.tsx | 328 ++++++++++++++++++ .../components/diffs/DiffFileEditButton.tsx | 310 +++++++++++++++++ .../src/components/files/FilePreviewPanel.tsx | 34 +- .../pullRequest/PullRequestCodeTab.tsx | 29 +- .../pullRequest/PullRequestDetailPanel.tsx | 1 + .../pullRequestEditing.logic.test.ts | 38 +- .../pullRequest/pullRequestEditing.logic.ts | 16 +- apps/web/src/routes/_chat.tsx | 2 + 9 files changed, 782 insertions(+), 23 deletions(-) create mode 100644 apps/web/src/components/diffs/DiffFileEditButton.test.tsx create mode 100644 apps/web/src/components/diffs/DiffFileEditButton.tsx diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 61b92ba266d6..fb3ad38584c7 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -1,6 +1,6 @@ import { RefreshIcon } from "~/components/ui/refresh-icon"; import { useAtomValue } from "@effect/atom-react"; -import type { FileDiffContentsLoader } from "@pierre/diffs"; +import type { FileDiffContentsLoader, FileDiffMetadata } from "@pierre/diffs"; import { useParams } from "@tanstack/react-router"; import { isAtomCommandInterrupted, @@ -27,7 +27,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useCodeViewFileReveal } from "./diffs/useCodeViewFileReveal"; import { useOpenInPreferredEditor } from "../editorPreferences"; import { type DraftId } from "../composerDraftStore"; -import { openDiffFilePrimaryAction } from "../diffFileActions"; +import { openDiffFilePrimaryAction, resolveDiffPathForWorkspace } from "../diffFileActions"; import { useCheckpointDiff } from "~/lib/checkpointDiffState"; import { cn } from "~/lib/utils"; import { selectThreadDiffPanelSelection, useDiffPanelStore } from "../diffPanelStore"; @@ -55,6 +55,7 @@ import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./Dif import { DiffStatLabel } from "./chat/DiffStatLabel"; import { AnnotatableCodeView, type AnnotatableCodeViewHandle } from "./diffs/AnnotatableCodeView"; import { DiffFileTree, type DiffFileTreeHandle } from "./diffs/DiffFileTree"; +import { DiffFileEditButton } from "./diffs/DiffFileEditButton"; import { diffFileTreeEntries } from "./diffs/diffFileTree.logic"; import { Button } from "./ui/button"; import { ToggleGroup, Toggle } from "./ui/toggle-group"; @@ -500,6 +501,44 @@ export default function DiffPanel({ }, [activeCwd, activeRepositoryRoot, openInPreferredEditor, routeThreadRef], ); + const editEnvironmentId = activeThread?.environmentId; + const renderHeaderFilenameSuffix = useCallback( + (fileDiff: FileDiffMetadata) => { + const filePath = resolveFileDiffPath(fileDiff); + const relativePath = resolveDiffPathForWorkspace({ + filePath, + workspaceRoot: activeCwd, + repositoryRoot: activeRepositoryRoot, + }); + return ( + <> + + {fileDiff.type !== "deleted" && + !shouldRetryBranchDiffAtEnvironmentCwd && + editEnvironmentId && + activeCwd && + relativePath ? ( + + ) : null} + + ); + }, + [ + activeCwd, + activeRepositoryRoot, + editEnvironmentId, + canRefreshGitDiff, + refreshBranchDiffPreview, + shouldRetryBranchDiffAtEnvironmentCwd, + ], + ); + const toggleDiffFileCollapsed = useCallback( (fileKey: string) => { setCollapsedDiffFiles((current) => { @@ -978,9 +1017,7 @@ export default function DiffPanel({ sectionId={reviewSectionId} sectionTitle={reviewSectionTitle} composerDraftTarget={composerDraftTarget} - renderHeaderFilenameSuffix={(fileDiff) => ( - - )} + renderHeaderFilenameSuffix={renderHeaderFilenameSuffix} renderHeaderPrefix={(fileDiff, fileKey, collapsed) => { const filePath = resolveFileDiffPath(fileDiff); return ( diff --git a/apps/web/src/components/diffs/DiffFileEditButton.test.tsx b/apps/web/src/components/diffs/DiffFileEditButton.test.tsx new file mode 100644 index 000000000000..d02275c2ca97 --- /dev/null +++ b/apps/web/src/components/diffs/DiffFileEditButton.test.tsx @@ -0,0 +1,328 @@ +import { EnvironmentId } from "@t3tools/contracts"; +import { act, type ReactNode } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; + +const { status, file, refresh, readFile, writeFile, blocker, blockOptions, dialogPayload } = + vi.hoisted(() => ({ + status: { + data: { refName: "review", pr: { url: "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/example/repo/pull/1" } }, + isSuccess: true, + isPending: false, + error: null as string | null, + }, + file: { data: { contents: "complete file", truncated: false }, error: null as string | null }, + refresh: vi.fn(), + readFile: vi.fn(), + writeFile: vi.fn(), + blockOptions: vi.fn(), + dialogPayload: { value: {} }, + blocker: { status: "idle", proceed: vi.fn(), reset: vi.fn() }, + })); +vi.mock("@t3tools/client-runtime/state/runtime", () => ({ executeAtomQuery: readFile })); +vi.mock("~/rpc/atomRegistry", () => ({ appAtomRegistry: { refresh } })); +vi.mock("~/hooks/useTheme", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) })); +vi.mock("~/hooks/useSettings", () => ({ useClientSettings: () => false })); +vi.mock("~/state/query", () => ({ + useEnvironmentQuery: () => status, + formatEnvironmentQueryError: () => "Save or read failed", +})); +vi.mock("~/state/projects", () => ({ projectEnvironment: { writeFile: {} } })); +vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => writeFile })); +vi.mock("@tanstack/react-router", () => ({ + useBlocker: (options: unknown) => { + blockOptions(options); + return blocker; + }, +})); +vi.mock("~/state/vcs", () => ({ vcsEnvironment: { status: () => null } })); +vi.mock("../files/projectFilesQueryState", () => ({ + getProjectFileQueryAtom: () => null, +})); +vi.mock("../DiffWorkerPoolProvider", () => ({ + DiffWorkerPoolProvider: ({ children }: { children: ReactNode }) => children, +})); +vi.mock("../files/FilePreviewPanel", () => ({ + EditableFileSurface: ({ + contents, + onContentsChange, + }: { + contents: string; + onContentsChange: (contents: string) => void; + }) =>