From de61c0c42b0797735a5cbe7ec6d4b3358fcf4295 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 2 Mar 2026 16:17:07 -0800 Subject: [PATCH] Guard worktree branch rename and fix branch toolbar state - Skip auto-rename when the active worktree branch no longer matches the temp branch - Update reactor tests to cover branch-switch and first-turn rename behavior - Show the actual current branch in the toolbar and use a VS Code icon in Open In picker --- .../Layers/ProviderCommandReactor.test.ts | 87 ++++++++++++-- .../Layers/ProviderCommandReactor.ts | 47 ++++++-- apps/web/src/components/BranchToolbar.tsx | 62 ++-------- apps/web/src/components/ChatView.tsx | 4 +- apps/web/src/components/Icons.tsx | 107 ++++++++++++++++++ 5 files changed, 231 insertions(+), 76 deletions(-) diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts index 3abe82b5440cf2c98fa179be106f1b84f95e93d7..a1213900c353fc60c0c272055a3fd21dfa93ce55 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts @@ -108,11 +108,27 @@ describe("ProviderCommandReactor", () => { const interruptTurn = vi.fn((_: unknown) => Effect.void); const respondToRequest = vi.fn((_: unknown) => Effect.void); const stopSession = vi.fn((_: unknown) => Effect.void); + const statusBranchByCwd = new Map(); const renameBranch = vi.fn((_: unknown) => Effect.succeed({ branch: "t3code/generated-name", }), ); + const statusDetails = vi.fn((cwd: string) => + Effect.succeed({ + branch: statusBranchByCwd.get(cwd) ?? null, + upstreamRef: null, + hasWorkingTreeChanges: false, + workingTree: { + files: [], + insertions: 0, + deletions: 0, + }, + hasUpstream: false, + aheadCount: 0, + behindCount: 0, + }), + ); const generateBranchName = vi.fn(() => Effect.succeed({ branch: "generated-name", @@ -141,7 +157,12 @@ describe("ProviderCommandReactor", () => { const layer = ProviderCommandReactorLive.pipe( Layer.provideMerge(orchestrationLayer), Layer.provideMerge(Layer.succeed(ProviderService, service)), - Layer.provideMerge(Layer.succeed(GitCore, { renameBranch } as unknown as GitCoreShape)), + Layer.provideMerge( + Layer.succeed( + GitCore, + { renameBranch, statusDetails } as unknown as GitCoreShape, + ), + ), Layer.provideMerge( Layer.succeed(TextGeneration, { generateBranchName } as unknown as TextGenerationShape), ), @@ -189,7 +210,11 @@ describe("ProviderCommandReactor", () => { respondToRequest, stopSession, renameBranch, + statusDetails, generateBranchName, + setStatusBranch: (cwd: string, branch: string | null) => { + statusBranchByCwd.set(cwd, branch); + }, stateDir, }; } @@ -235,6 +260,7 @@ describe("ProviderCommandReactor", () => { it("generates and renames temporary worktree branch on first turn", async () => { const harness = await createHarness(); const now = new Date().toISOString(); + const worktreeCwd = "/tmp/provider-project/.t3/worktrees/t3code-89abc123"; await Effect.runPromise( harness.engine.dispatch({ @@ -242,9 +268,10 @@ describe("ProviderCommandReactor", () => { commandId: CommandId.makeUnsafe("cmd-thread-meta-set-temp-branch"), threadId: ThreadId.makeUnsafe("thread-1"), branch: "t3code/89abc123", - worktreePath: "/tmp/provider-project/.t3/worktrees/t3code-89abc123", + worktreePath: worktreeCwd, }), ); + harness.setStatusBranch(worktreeCwd, "t3code/89abc123"); await Effect.runPromise( harness.engine.dispatch({ @@ -275,7 +302,7 @@ describe("ProviderCommandReactor", () => { await waitFor(() => harness.renameBranch.mock.calls.length === 1); expect(harness.generateBranchName.mock.calls[0]?.[0]).toEqual({ - cwd: "/tmp/provider-project/.t3/worktrees/t3code-89abc123", + cwd: worktreeCwd, message: "Fix visual bug from screenshot", attachments: [ { @@ -288,18 +315,14 @@ describe("ProviderCommandReactor", () => { ], }); expect(harness.renameBranch.mock.calls[0]?.[0]).toEqual({ - cwd: "/tmp/provider-project/.t3/worktrees/t3code-89abc123", + cwd: worktreeCwd, oldBranch: "t3code/89abc123", newBranch: "t3code/generated-name", }); - await waitFor(() => { - const readModel = Effect.runSync(harness.engine.getReadModel()); - const thread = readModel.threads.find( - (entry) => entry.id === ThreadId.makeUnsafe("thread-1"), - ); - return thread?.branch === "t3code/generated-name"; - }); + const readModel = await Effect.runPromise(harness.engine.getReadModel()); + const thread = readModel.threads.find((entry) => entry.id === ThreadId.makeUnsafe("thread-1")); + expect(thread?.branch).toBe("t3code/89abc123"); }); it("passes persisted attachment references to branch generation and turn start", async () => { @@ -373,6 +396,7 @@ describe("ProviderCommandReactor", () => { it("skips worktree branch generation after the first user turn", async () => { const harness = await createHarness(); const now = new Date().toISOString(); + const worktreeCwd = "/tmp/provider-project/.t3/worktrees/t3code-1234abcd"; await Effect.runPromise( harness.engine.dispatch({ @@ -380,9 +404,10 @@ describe("ProviderCommandReactor", () => { commandId: CommandId.makeUnsafe("cmd-thread-meta-set-temp-branch-2"), threadId: ThreadId.makeUnsafe("thread-1"), branch: "t3code/1234abcd", - worktreePath: "/tmp/provider-project/.t3/worktrees/t3code-1234abcd", + worktreePath: worktreeCwd, }), ); + harness.setStatusBranch(worktreeCwd, "t3code/1234abcd"); await Effect.runPromise( harness.engine.dispatch({ @@ -425,6 +450,44 @@ describe("ProviderCommandReactor", () => { expect(harness.renameBranch.mock.calls.length).toBe(1); }); + it("skips worktree rename when active branch already changed", async () => { + const harness = await createHarness(); + const now = new Date().toISOString(); + const worktreeCwd = "/tmp/provider-project/.t3/worktrees/t3code-badf00d0"; + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.meta.update", + commandId: CommandId.makeUnsafe("cmd-thread-meta-set-temp-branch-switched"), + threadId: ThreadId.makeUnsafe("thread-1"), + branch: "t3code/badf00d0", + worktreePath: worktreeCwd, + }), + ); + harness.setStatusBranch(worktreeCwd, "feature/add-vscode-open-in-support"); + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.turn.start", + commandId: CommandId.makeUnsafe("cmd-turn-start-worktree-switched"), + threadId: ThreadId.makeUnsafe("thread-1"), + message: { + messageId: asMessageId("user-message-worktree-switched"), + role: "user", + text: "Fix visual bug from screenshot", + attachments: [], + }, + approvalPolicy: "on-request", + sandboxMode: "workspace-write", + createdAt: now, + }), + ); + + await waitFor(() => harness.generateBranchName.mock.calls.length === 1); + await Effect.runPromise(Effect.sleep("20 millis")); + expect(harness.renameBranch.mock.calls.length).toBe(0); + }); + it("skips worktree rename when branch-name generation fails", async () => { const harness = await createHarness(); const now = new Date().toISOString(); diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index bf88e221b91c2ecec8e5fe9cd04aa0859b4ce8cc..87b63b452315374e32dcb8c5e2dd1c38c77f041e 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -330,17 +330,42 @@ const make = Effect.gen(function* () { const targetBranch = buildGeneratedWorktreeBranchName(generated.branch); if (targetBranch === oldBranch) return Effect.void; - return Effect.flatMap( - git.renameBranch({ cwd, oldBranch, newBranch: targetBranch }), - (renamed) => - orchestrationEngine.dispatch({ - type: "thread.meta.update", - commandId: serverCommandId("worktree-branch-rename"), - threadId: input.threadId, - branch: renamed.branch, - worktreePath: cwd, - }), - ); + return Effect.gen(function* () { + const currentBranch = yield* git.statusDetails(cwd).pipe( + Effect.map((status) => status.branch), + Effect.catch((error) => + Effect.logWarning( + "provider command reactor failed to read active worktree branch", + { + threadId: input.threadId, + cwd, + oldBranch, + targetBranch, + reason: error.message, + }, + ).pipe(Effect.as(null)), + ), + ); + + if (currentBranch !== oldBranch) { + return yield* Effect.logWarning( + "provider command reactor skipped worktree branch rename because active branch changed", + { + threadId: input.threadId, + cwd, + oldBranch, + targetBranch, + currentBranch, + }, + ); + } + + yield* git.renameBranch({ + cwd, + oldBranch, + newBranch: targetBranch, + }); + }); }), Effect.catchCause((cause) => Effect.logWarning( diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index 7d1317af142920f0ceffd949ed1ca60455f84714..76f802a86df79d457eff25e84a54d08b4158f66f 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -1,6 +1,6 @@ import type { GitBranch, ThreadId } from "@t3tools/contracts"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { useEffect, useState } from "react"; +import { useState } from "react"; import { newCommandId } from "../lib/utils"; import { @@ -14,7 +14,6 @@ import { useStore } from "../store"; import { dedupeRemoteBranchesWithLocalMatches, deriveLocalBranchNameFromRemoteRef, - deriveSyncedLocalBranch, } from "./BranchToolbar.logic"; import { Button } from "./ui/button"; import { @@ -72,6 +71,11 @@ export default function BranchToolbar({ const branchesQuery = useQuery(gitBranchesQueryOptions(branchCwd)); const branches = dedupeRemoteBranchesWithLocalMatches(branchesQuery.data?.branches ?? []); + const currentGitBranch = branches.find((branch) => branch.current)?.name ?? null; + const resolvedActiveBranch = + effectiveEnvMode === "worktree" && !activeWorktreePath + ? activeThreadBranch + : (currentGitBranch ?? activeThreadBranch); const branchNames = branches.map((branch) => branch.name); const branchByName = new Map(branches.map((branch) => [branch.name, branch])); const trimmedBranchQuery = branchQuery.trim(); @@ -92,50 +96,6 @@ export default function BranchToolbar({ gitCreateBranchAndCheckoutMutationOptions({ cwd: branchCwd, queryClient }), ); - // ── Effects ─────────────────────────────────────────────────────────── - - // Keep thread branch synced to git current branch for local threads. - const queryBranches = branchesQuery.data?.branches; - useEffect(() => { - const syncedBranch = deriveSyncedLocalBranch({ - activeThreadId, - activeWorktreePath, - envMode: effectiveEnvMode, - activeThreadBranch, - queryBranches, - }); - if (!activeThreadId || !syncedBranch) return; - const api = readNativeApi(); - - if (api && hasServerThread) { - void api.orchestration.dispatchCommand({ - type: "thread.meta.update", - commandId: newCommandId(), - threadId: activeThreadId, - branch: syncedBranch, - worktreePath: null, - }); - } - if (hasServerThread) { - setThreadBranchAction(activeThreadId, syncedBranch, null); - return; - } - setDraftThreadContext(threadId, { - branch: syncedBranch, - worktreePath: null, - }); - }, [ - activeThreadId, - activeWorktreePath, - activeThreadBranch, - queryBranches, - effectiveEnvMode, - setThreadBranchAction, - hasServerThread, - setDraftThreadContext, - threadId, - ]); - // ── Helpers ─────────────────────────────────────────────────────────── const setThreadError = (error: string | null) => { @@ -282,7 +242,7 @@ export default function BranchToolbar({ } }} open={isBranchMenuOpen} - value={activeThreadBranch} + value={resolvedActiveBranch} > } @@ -290,10 +250,10 @@ export default function BranchToolbar({ disabled={branchesQuery.isLoading} > - {activeThreadBranch + {resolvedActiveBranch ? effectiveEnvMode === "worktree" && !activeWorktreePath - ? `From ${activeThreadBranch}` - : activeThreadBranch + ? `From ${resolvedActiveBranch}` + : resolvedActiveBranch : "Select branch"} @@ -347,7 +307,7 @@ export default function BranchToolbar({ key={itemValue} value={itemValue} className={ - itemValue === activeThreadBranch ? "bg-accent text-foreground" : undefined + itemValue === resolvedActiveBranch ? "bg-accent text-foreground" : undefined } onClick={() => selectBranch(branch)} > diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 42accea48010ac22b85c2323c4f0a672d9e459b7..df5f5fd1de751db1fa53c5c77161b095e01f170f 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -100,7 +100,7 @@ import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "./u import { Separator } from "./ui/separator"; import { Group, GroupSeparator } from "./ui/group"; import { Menu, MenuItem, MenuPopup, MenuShortcut, MenuTrigger } from "./ui/menu"; -import { CursorIcon, Icon } from "./Icons"; +import { CursorIcon, Icon, VisualStudioCode } from "./Icons"; import { cn, isMacPlatform, isWindowsPlatform } from "~/lib/utils"; import { Badge } from "./ui/badge"; import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip"; @@ -3681,7 +3681,7 @@ const OpenInPicker = memo(function OpenInPicker({ }, { label: "VS Code", - Icon: FileIcon, + Icon: VisualStudioCode, value: "vscode", }, { diff --git a/apps/web/src/components/Icons.tsx b/apps/web/src/components/Icons.tsx index 93e530786bfff94ada04a346baf0c02fcd8f6832..70cc5a1d2551b0bb01f7b928c0cd29316b79a17b 100644 --- a/apps/web/src/components/Icons.tsx +++ b/apps/web/src/components/Icons.tsx @@ -19,3 +19,110 @@ export const CursorIcon: Icon = (props) => ( ); + +export const VisualStudioCode: Icon = (props) => ( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +); -- 2.54.0