From b4620d595554654c259ec7c0afb3d3d5532959a8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 16:41:05 -0700 Subject: [PATCH] fix(server): reject file rewind in shared workspaces (#12306) Co-authored-by: Claude Fable 5 --- .../Layers/CheckpointReactor.test.ts | 112 ++++++++++++++++-- .../orchestration/Layers/CheckpointReactor.ts | 64 ++++++++++ apps/web/src/components/ChatView.tsx | 25 ++-- docs/user/composer.md | 5 +- 4 files changed, 186 insertions(+), 20 deletions(-) diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index 2cc7a4399915e5c4b511df0c082b314e87a9dabe..7091055cd2c21ce59a9e81f7d93e4593dd2607b6 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -297,6 +297,7 @@ describe("CheckpointReactor", () => { readonly threadWorktreePath?: string | null; readonly threadBranch?: string | null; readonly secondThreadSharingWorktree?: boolean; + readonly secondThreadWorktreePath?: (cwd: string) => string; readonly localStatusRefName?: string | null; readonly providerSessionCwd?: string; readonly providerName?: ProviderDriverKind; @@ -436,7 +437,8 @@ describe("CheckpointReactor", () => { interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, runtimeMode: "approval-required", branch: options?.threadBranch ?? null, - worktreePath: options?.threadWorktreePath ?? cwd, + worktreePath: + options?.threadWorktreePath !== undefined ? options.threadWorktreePath : cwd, createdAt, }) .pipe( @@ -455,7 +457,8 @@ describe("CheckpointReactor", () => { interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, runtimeMode: "approval-required", branch: null, - worktreePath: options?.threadWorktreePath ?? cwd, + worktreePath: + options?.secondThreadWorktreePath?.(cwd) ?? options?.threadWorktreePath ?? cwd, createdAt, }), ) @@ -497,6 +500,81 @@ describe("CheckpointReactor", () => { }; } + effectIt.effect.each([ + "active", + "archived", + "alias", + "nested", + "ancestor", + "project-root", + "conversation", + ] as const)("preserves sibling files when reverting a shared workspace, owner=%s", (owner) => + Effect.gen(function* () { + const harness = yield* Effect.promise(() => + createHarness({ + secondThreadSharingWorktree: true, + ...(owner === "alias" || owner === "nested" || owner === "ancestor" + ? { + secondThreadWorktreePath: (cwd: string) => { + if (owner === "ancestor") return NodePath.dirname(cwd); + if (owner === "nested") { + const nested = NodePath.join(cwd, "nested-owner"); + NodeFS.mkdirSync(nested); + return nested; + } + const alias = `${cwd}-alias`; + NodeFS.symlinkSync(cwd, alias, "junction"); + tempDirs.push(alias); + return alias; + }, + } + : {}), + }), + ); + const createdAt = "2026-01-01T00:00:02.000Z"; + if (owner === "archived") + yield* harness.engine.dispatch({ + type: "thread.archive", + commandId: CommandId.make("cmd-archive-owner"), + threadId: ThreadId.make("thread-2"), + }); + if (owner === "project-root") + yield* harness.engine.dispatch({ + type: "thread.meta.update", + commandId: CommandId.make("cmd-root-owner"), + threadId: ThreadId.make("thread-2"), + worktreePath: null, + }); + const siblingFile = NodePath.join( + harness.cwd, + ...(owner === "nested" ? ["nested-owner"] : []), + "sibling-work.txt", + ); + NodeFS.writeFileSync(siblingFile, "sibling work\n"); + yield* harness.engine.dispatch({ + type: owner === "conversation" ? "thread.conversation.revert" : "thread.checkpoint.revert", + commandId: CommandId.make("cmd-shared-revert"), + threadId: ThreadId.make("thread-1"), + turnCount: 0, + createdAt, + }); + yield* Effect.promise(harness.drain); + expect(NodeFS.readFileSync(siblingFile, "utf8")).toBe("sibling work\n"); + expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v3\n"); + const model = yield* Effect.promise(harness.readModel); + const failure = model.threads + .find((t) => t.id === "thread-1") + ?.activities.find((a) => a.kind === "checkpoint.revert.failed"); + if (owner === "conversation") expect(failure).toBeUndefined(); + else { + expect(failure?.payload).toMatchObject({ + detail: expect.stringContaining("isolated worktree"), + }); + expect(harness.provider.rollbackConversation).not.toHaveBeenCalled(); + } + }), + ); + effectIt.effect("captures baseline and large turn summaries before completion receipts", () => Effect.gen(function* () { const harness = yield* Effect.promise(() => @@ -1963,7 +2041,7 @@ describe("CheckpointReactor", () => { }); it.each([false, true])( - "reverts without an active session using project cwd fallback: %s", + "restores files only in an isolated worktree without an active session, project cwd=%s", async (useProjectCwd) => { const harness = await createHarness({ hasSession: false, @@ -1995,12 +2073,28 @@ describe("CheckpointReactor", () => { }), ); - await waitForEvent(harness.engine, (event) => event.type === "thread.reverted"); - expect(harness.provider.rollbackConversation).toHaveBeenCalledWith({ - threadId: ThreadId.make("thread-1"), - numTurns: 1, - }); - expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v1\n"); + await harness.drain(); + if (useProjectCwd) { + expect(harness.provider.rollbackConversation).not.toHaveBeenCalled(); + expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v3\n"); + const model = await harness.readModel(); + expect(model.threads[0]?.activities).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "checkpoint.revert.failed", + payload: expect.objectContaining({ + detail: expect.stringContaining("isolated worktree"), + }), + }), + ]), + ); + } else { + expect(harness.provider.rollbackConversation).toHaveBeenCalledWith({ + threadId: ThreadId.make("thread-1"), + numTurns: 1, + }); + expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v1\n"); + } }, ); }); diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.ts index d0d867fdd25e9fe1047fc973756f55608efde3ef..822df29d0f633031032e0919120fb07ac138a20e 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.ts @@ -15,6 +15,8 @@ import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; 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 Option from "effect/Option"; import type * as PlatformError from "effect/PlatformError"; import * as Stream from "effect/Stream"; @@ -87,6 +89,8 @@ const make = Effect.gen(function* () { const checkpointStore = yield* CheckpointStore.CheckpointStore; const receiptBus = yield* RuntimeReceiptBus; const workspaceEntries = yield* WorkspaceEntries.WorkspaceEntries; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; const vcsStatusBroadcaster = yield* VcsStatusBroadcaster; const pullRequests = yield* PullRequestService.PullRequestService; const startedTurns = new Map(); @@ -687,6 +691,55 @@ const make = Effect.gen(function* () { }); }); + // Checkpoints contain the whole checkout, so restoring a shared cwd can erase a sibling's work. + const isRestoreWorkspaceIsolated = Effect.fn("isRestoreWorkspaceIsolated")(function* ( + thread: { readonly id: ThreadId; readonly worktreePath: string | null }, + cwd: string, + ) { + if (thread.worktreePath === null) return false; + const canonicalCwd = yield* fileSystem.realPath(cwd); + if ((yield* fileSystem.realPath(thread.worktreePath)) !== canonicalCwd) return false; + const active = yield* projectionSnapshotQuery.getShellSnapshot(); + const archived = yield* projectionSnapshotQuery.getArchivedShellSnapshot(); + const projects = [...active.projects, ...archived.projects]; + const paths = new Set(); + for (const other of [...active.threads, ...archived.threads]) { + if (other.id === thread.id) continue; + const candidate = + other.worktreePath ?? + projects.find((project) => project.id === other.projectId)?.workspaceRoot; + if (candidate !== undefined) paths.add(candidate); + } + for (const session of yield* providerService.listSessions()) { + if ( + session.threadId !== thread.id && + session.status !== "closed" && + session.cwd !== undefined + ) + paths.add(session.cwd); + } + for (const candidate of paths) { + const otherCwd = yield* fileSystem + .realPath(candidate) + .pipe( + Effect.catch((error) => + error.reason._tag === "NotFound" ? Effect.succeed(null) : Effect.fail(error), + ), + ); + if (otherCwd === null) continue; + const isWithin = (parent: string, child: string) => { + const relative = path.relative(parent, child); + return ( + relative === "" || + (!path.isAbsolute(relative) && relative !== ".." && !relative.startsWith(`..${path.sep}`)) + ); + }; + // Parent and nested owners can both have files inside the restore target. + if (isWithin(canonicalCwd, otherCwd) || isWithin(otherCwd, canonicalCwd)) return false; + } + return true; + }); + const handleRevertRequested = Effect.fn("handleRevertRequested")(function* ( event: Extract, ) { @@ -742,6 +795,17 @@ const make = Effect.gen(function* () { return; } + if (!(yield* isRestoreWorkspaceIsolated(thread, checkpointCwd))) { + yield* appendRevertFailureActivity({ + threadId: thread.id, + turnCount: event.payload.turnCount, + detail: + "File restore requires an isolated worktree. This workspace may contain changes from another thread. Rewind the conversation without restoring files instead.", + createdAt: now, + }).pipe(Effect.catch(() => Effect.void)); + return; + } + const targetCheckpointRef = event.payload.turnCount === 0 ? checkpointRefForThreadTurn(event.payload.threadId, 0) diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 267d420c9d44052995da53ed025c8c5aed16cff8..cfde95e491335ee43006bf0029ddf16a0eb932c4 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -10393,20 +10393,25 @@ export default function ChatView(props: ChatViewProps) { Rewind chat to before this message. Your prompt and attachments return to the composer. + {activeWorktreePath === null + ? " Files stay as they are because this thread shares the project directory." + : null} }>Cancel - + {activeWorktreePath !== null ? ( + + ) : null}