diff --git a/CONTEXT.md b/CONTEXT.md index 4f1af6a..1e4027c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -43,6 +43,10 @@ _Avoid_: session, chat, conversation (as an entity name), committed thread Controller-local composer state before a thread exists: the chosen project, branch or worktree choice, agent and preferences, and the unsent message. A draft never leaves the controller. _Avoid_: draft thread +**Worktree**: +A second git checkout of a project, on its own freshly-forked branch, that a thread runs from instead of the project directory — created by forking a new branch off the requested ref rather than checking that ref out directly, since a ref can only be checked out in one worktree at a time. +_Avoid_: clone, sandbox + **Bind**: The worker-internal act of making a thread's persisted session live — creating it at the thread's first message or resuming it on demand — yielding that session's catalog. _Avoid_: attach, connect diff --git a/apps/web/src/components/chat/composer/composer-branch-toolbar.test.tsx b/apps/web/src/components/chat/composer/composer-branch-toolbar.test.tsx new file mode 100644 index 0000000..c98efae --- /dev/null +++ b/apps/web/src/components/chat/composer/composer-branch-toolbar.test.tsx @@ -0,0 +1,131 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { toast } from "sonner"; +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { ComposerBranchToolbar } from "./composer-branch-toolbar"; + +const useGitStatusMock = vi.fn(); +const useProjectGitStatusMock = vi.fn(); +const useListGitRefsMock = vi.fn(); +const useListProjectGitRefsMock = vi.fn(); +const useCheckoutRefMock = vi.fn(); +const useCreateWorktreeMock = vi.fn(); + +vi.mock("@cyrus/hooks/queries/use-git", () => ({ + useGitStatus: (arg: unknown) => useGitStatusMock(arg), + useProjectGitStatus: (arg: unknown) => useProjectGitStatusMock(arg), + useListGitRefs: (arg: unknown) => useListGitRefsMock(arg), + useListProjectGitRefs: (arg: unknown) => useListProjectGitRefsMock(arg), + useCheckoutRef: () => useCheckoutRefMock(), + useCreateWorktree: () => useCreateWorktreeMock(), +})); + +vi.mock("@cyrus/hooks/stores/local-draft", () => ({ + useLocalDraftStore: ( + selector: (state: { + gitByDraft: Record; + setBranch: () => void; + setWorktree: () => void; + }) => unknown + ) => + selector({ + gitByDraft: {}, + setBranch: vi.fn(), + setWorktree: vi.fn(), + }), +})); + +vi.mock("sonner", () => ({ toast: { error: vi.fn() } })); + +function idleQuery() { + return { data: undefined, isLoading: false }; +} + +function idleMutation() { + return { mutate: vi.fn(), reset: vi.fn(), error: null, isPending: false }; +} + +beforeEach(() => { + vi.clearAllMocks(); + useGitStatusMock.mockReturnValue({ + data: { isRepo: true, refName: "main" }, + }); + useProjectGitStatusMock.mockReturnValue({ + data: { isRepo: true, refName: "main" }, + }); + useListGitRefsMock.mockReturnValue(idleQuery()); + useListProjectGitRefsMock.mockReturnValue(idleQuery()); + useCheckoutRefMock.mockReturnValue(idleMutation()); + useCreateWorktreeMock.mockReturnValue(idleMutation()); +}); + +describe("ComposerBranchToolbar", () => { + test("does not fetch the ref list until the branch dropdown opens", async () => { + const user = userEvent.setup(); + render( + + ); + + expect(useListGitRefsMock).toHaveBeenLastCalledWith(undefined); + + await user.click(screen.getByRole("button", { name: "main" })); + + expect(useListGitRefsMock).toHaveBeenLastCalledWith("thread-1"); + }); + + test("defers the project ref list for drafts until the dropdown opens", async () => { + const user = userEvent.setup(); + render( + + ); + + expect(useListProjectGitRefsMock).toHaveBeenLastCalledWith(undefined); + + await user.click(screen.getByRole("button", { name: "main" })); + + expect(useListProjectGitRefsMock).toHaveBeenLastCalledWith("project-1"); + }); + + test("locks the workspace selector for committed threads regardless of worktree state", () => { + render( + + ); + + expect(screen.queryByRole("combobox")).not.toBeInTheDocument(); + expect(screen.getByText("Current checkout")).toBeInTheDocument(); + }); + + test("keeps the workspace selector editable for drafts", () => { + render( + + ); + + expect(screen.getByRole("combobox")).toBeInTheDocument(); + }); + + test("toasts a branch checkout conflict instead of rendering it inline", () => { + useCheckoutRefMock.mockReturnValue({ + mutate: vi.fn(), + reset: vi.fn(), + error: { message: "Branch 'fix' is already checked out at /path" }, + isPending: false, + }); + + render( + + ); + + expect(toast.error).toHaveBeenCalledWith( + "Branch 'fix' is already checked out at /path" + ); + expect( + screen.queryByText("Branch 'fix' is already checked out at /path") + ).not.toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/components/chat/composer/composer-branch-toolbar.tsx b/apps/web/src/components/chat/composer/composer-branch-toolbar.tsx index c4a8ee2..834eca0 100644 --- a/apps/web/src/components/chat/composer/composer-branch-toolbar.tsx +++ b/apps/web/src/components/chat/composer/composer-branch-toolbar.tsx @@ -17,6 +17,7 @@ import { SearchIcon, } from "lucide-react"; import { useEffect, useMemo, useState } from "react"; +import { toast } from "sonner"; import { DropdownMenu, DropdownMenuContent, @@ -74,6 +75,20 @@ function resolveBranchTriggerLabel( return refName; } +function refsQueryId( + isActiveSource: boolean, + branchMenuOpened: boolean, + id: string +): string | undefined { + return isActiveSource && branchMenuOpened ? id : undefined; +} + +function useMutationErrorToast(error: Error | null | undefined) { + useEffect(() => { + if (error) toast.error(error.message); + }, [error]); +} + function BranchListItems({ filteredRefs, isLoading, @@ -116,9 +131,12 @@ export function ComposerBranchToolbar({ localDraft ? subject.projectId : undefined ); const gitStatus = localDraft ? projectGitStatus : threadGitStatus; - const threadGitRefs = useListGitRefs(localDraft ? undefined : subject.id); + const [branchMenuOpened, setBranchMenuOpened] = useState(false); + const threadGitRefs = useListGitRefs( + refsQueryId(!localDraft, branchMenuOpened, subject.id) + ); const projectGitRefs = useListProjectGitRefs( - localDraft ? subject.projectId : undefined + refsQueryId(localDraft, branchMenuOpened, subject.projectId) ); const gitRefs = localDraft ? projectGitRefs : threadGitRefs; const checkoutRef = useCheckoutRef(); @@ -150,7 +168,7 @@ export function ComposerBranchToolbar({ ? (draftGit?.branch ?? statusRefName) : statusRefName; const hasWorktree = localDraft ? false : Boolean(subject.worktreePath); - const envLocked = hasWorktree; + const envLocked = !localDraft; const branchMutationError = localDraft ? null : (checkoutRef.error ?? createWorktree.error); @@ -165,6 +183,8 @@ export function ComposerBranchToolbar({ const refsLoading = gitRefs.isLoading && gitRefs.data === undefined; + useMutationErrorToast(branchMutationError); + if (!isRepo) return null; function handleBranchSelect(name: string) { @@ -233,7 +253,11 @@ export function ComposerBranchToolbar({ { - if (!open) setBranchQuery(""); + if (open) { + setBranchMenuOpened(true); + } else { + setBranchQuery(""); + } }} > - - {branchMutationError ? ( - - {branchMutationError.message} - - ) : null} ); } diff --git a/apps/web/src/components/chat/composer/composer-lower-chrome.test.tsx b/apps/web/src/components/chat/composer/composer-lower-chrome.test.tsx new file mode 100644 index 0000000..53fd0c0 --- /dev/null +++ b/apps/web/src/components/chat/composer/composer-lower-chrome.test.tsx @@ -0,0 +1,31 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, test, vi } from "vitest"; +import { ComposerLowerChrome } from "./composer-lower-chrome"; + +vi.mock("@/components/chat/composer/composer-branch-toolbar", () => ({ + ComposerBranchToolbar: () => ( +
branch toolbar
+ ), +})); + +const subject = { id: "draft-1", projectId: "p1" }; +const BRANCH_WORKTREE_TOGGLE_NAME = /branch.*worktree/i; + +describe("ComposerLowerChrome", () => { + test("renders the branch toolbar directly for a git-repo draft, with no intermediate toggle", () => { + render(); + + expect(screen.getByTestId("branch-toolbar")).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: BRANCH_WORKTREE_TOGGLE_NAME }) + ).not.toBeInTheDocument(); + }); + + test("renders nothing when the project isn't a git repo", () => { + render( + + ); + + expect(screen.queryByTestId("branch-toolbar")).not.toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/components/chat/composer/composer-lower-chrome.tsx b/apps/web/src/components/chat/composer/composer-lower-chrome.tsx index 4c3688f..fc4ab9f 100644 --- a/apps/web/src/components/chat/composer/composer-lower-chrome.tsx +++ b/apps/web/src/components/chat/composer/composer-lower-chrome.tsx @@ -1,18 +1,13 @@ import { cn } from "cnfast"; import { ComposerBranchToolbar } from "@/components/chat/composer/composer-branch-toolbar"; -import { Button } from "@/components/ui/button"; import type { ComposerSubject } from "@/types/composer"; export function ComposerLowerChrome({ localDraft, - draftGitOpen, - onOpenDraftGit, isGitRepo, subject, }: { localDraft: boolean; - draftGitOpen: boolean; - onOpenDraftGit: () => void; isGitRepo: boolean; subject: ComposerSubject; }) { @@ -25,19 +20,6 @@ export function ComposerLowerChrome({ : "pb-[calc(env(safe-area-inset-bottom)+0.75rem)] sm:pb-[calc(env(safe-area-inset-bottom)+1rem)]" )} > - {localDraft && !draftGitOpen ? ( -
- -
- ) : null} {isGitRepo ? ( { - setDraftGitOpen(false); - }, [threadId, projectId]); - // biome-ignore lint/correctness/useExhaustiveDependencies: reset only on identity change useEffect(() => { setDraftCatalogSettled(false); }, [threadId, projectId]); @@ -108,7 +103,7 @@ export function Composer({ ]); const threadGitStatus = useGitStatus(localDraft ? undefined : threadId); const projectGitStatus = useProjectGitStatus( - localDraft && draftGitOpen ? projectId : undefined + localDraft ? projectId : undefined ); const isGitRepo = (localDraft ? projectGitStatus : threadGitStatus).data?.isRepo === true; @@ -301,10 +296,8 @@ export function Composer({ setDraftGitOpen(true)} subject={subject} /> diff --git a/docs/adr/0022-thread-workspace-frozen-at-commit-never-shared.md b/docs/adr/0022-thread-workspace-frozen-at-commit-never-shared.md new file mode 100644 index 0000000..c015701 --- /dev/null +++ b/docs/adr/0022-thread-workspace-frozen-at-commit-never-shared.md @@ -0,0 +1,9 @@ +# A thread's workspace is frozen at commit and never shared with another thread + +_Decided 2026-08-06, while fixing issue [#146](https://github.com/soorya-u/cyrus/issues/146)._ + +Once a thread is committed, its workspace choice — current checkout vs. worktree, and which worktree — is frozen forever: the composer's workspace-mode control locks on `!localDraft`, not on `hasWorktree` as it did before. This removes a previously-working capability: a committed thread sitting on "current checkout" could be converted into a worktree after the fact by picking "New worktree" from its (until now unlocked) workspace select. Separately, creating a worktree always forks a fresh, isolated branch (existing `createGitWorktree` behavior) — a thread is never pointed at a worktree another thread already owns. + +We looked at how `pingdotgg/t3code` handles the equivalent selector, since it's the app this UI is being modeled on (verified 2026-08-06 against a fresh clone; behavior may drift upstream). It locks only its workspace-mode *dropdown* once a thread has messages or a running session (`apps/web/src/components/ChatView.tsx`'s `envLocked` and `BranchToolbar.tsx`'s `envModeLocked`) — but its branch picker can still silently redirect a committed thread's `worktreePath` underneath that lock via `updateThreadMetadata` (`apps/web/src/components/BranchToolbarBranchSelector.tsx`), and it offers reusing another thread's existing worktree via a "Previous worktree" option (`resolvePreviousWorktreeSeed` in `apps/web/src/components/BranchToolbar.logic.ts`). Under cyrus's born-committed model ([0015](./0015-drafts-are-controller-local-threads-born-committed.md)) a committed thread always has a first message, so t3code's activity-based lock collapses to `!localDraft` for us regardless — that much isn't a real deviation. The reuse case is: neither codebase has any mechanism to stop two threads from running their agents concurrently in one shared working directory — simultaneous file writes and git index-lock contention, unattributable in the diff panel. Rather than build that exclusivity guarantee, we removed the ability to ever repoint or share a thread's workspace after commit, closing the failure mode instead of managing it. + +Considered and rejected: mirroring t3code's model (branch-picker-driven `worktreePath` changes post-commit, plus worktree reuse for new threads) — rejected because it reintroduces the shared-cwd race that neither implementation actually guards against.