diff --git a/apps/web/components/task-create-dialog-branch-utils.test.ts b/apps/web/components/task-create-dialog-branch-utils.test.ts new file mode 100644 index 0000000000..c414888a93 --- /dev/null +++ b/apps/web/components/task-create-dialog-branch-utils.test.ts @@ -0,0 +1,189 @@ +import { describe, it, expect } from "vitest"; +import { + computeBranchPrefix, + computeBranchTooltip, + computeBranchDisabledReason, +} from "./task-create-dialog-branch-utils"; + +describe("computeBranchPrefix", () => { + it("returns empty when no branch is picked yet", () => { + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "", + currentLocalBranch: "main", + freshBranchEnabled: false, + }), + ).toBe(""); + }); + + it("returns 'current: ' for local executor when row branch matches workspace current", () => { + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "main", + currentLocalBranch: "main", + freshBranchEnabled: false, + }), + ).toBe("current: "); + }); + + it("returns 'current: ' on detached HEAD when the picked value is the same short SHA", () => { + // Backend returns a short SHA in `current_branch` when the repo is on + // detached HEAD; the autoselect seeds row.branch with that same value + // so the chip can read "current: " instead of falling through to + // "will switch to: master". + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "4fbc5d7", + currentLocalBranch: "4fbc5d7", + freshBranchEnabled: false, + }), + ).toBe("current: "); + }); + + it("returns 'will switch to: ' for local executor when row branch differs from current", () => { + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "develop", + currentLocalBranch: "main", + freshBranchEnabled: false, + }), + ).toBe("will switch to: "); + }); + + it("treats unknown current branch as 'will switch to' on local executor", () => { + // Local-status fetch error leaves currentLocalBranch="". With no baseline + // we can't claim the picked branch is "current", so fall through to the + // destructive label. + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "main", + currentLocalBranch: "", + freshBranchEnabled: false, + }), + ).toBe("will switch to: "); + }); + + it("returns 'from: ' for non-local executors regardless of current branch", () => { + expect( + computeBranchPrefix({ + isLocalExecutor: false, + rowBranch: "main", + currentLocalBranch: "main", + freshBranchEnabled: false, + }), + ).toBe("from: "); + expect( + computeBranchPrefix({ + isLocalExecutor: false, + rowBranch: "develop", + currentLocalBranch: "", + freshBranchEnabled: false, + }), + ).toBe("from: "); + }); + + it("returns 'from: ' when fork-a-new-branch is enabled, even on local executor", () => { + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "main", + currentLocalBranch: "main", + freshBranchEnabled: true, + }), + ).toBe("from: "); + expect( + computeBranchPrefix({ + isLocalExecutor: true, + rowBranch: "develop", + currentLocalBranch: "main", + freshBranchEnabled: true, + }), + ).toBe("from: "); + }); +}); + +describe("computeBranchTooltip", () => { + it('returns the "current" tooltip for "current: " prefix', () => { + expect(computeBranchTooltip("current: ")).toContain("as-is"); + }); + + it('returns the "switch" tooltip for "will switch to: " prefix', () => { + expect(computeBranchTooltip("will switch to: ")).toContain("git checkout"); + }); + + it('returns the "fork" tooltip for "from: " prefix', () => { + expect(computeBranchTooltip("from: ")).toContain("forked"); + }); + + it("returns a generic tooltip for empty or undefined prefix", () => { + expect(computeBranchTooltip("")).toContain("Branch the agent will run against"); + expect(computeBranchTooltip(undefined)).toContain("Branch the agent will run against"); + }); + + it("returns a generic tooltip for an unrecognized prefix", () => { + // Defensive: unknown prefix shouldn't crash, just falls back. + expect(computeBranchTooltip("foo: ")).toContain("Branch the agent will run against"); + }); +}); + +describe("computeBranchDisabledReason", () => { + it("returns the lock reason when branchLocked is true", () => { + expect( + computeBranchDisabledReason({ + branchLocked: true, + hasRepo: true, + branchesLoading: false, + optionCount: 5, + }), + ).toContain("local executor"); + }); + + it("asks to select a repo first when none is set", () => { + expect( + computeBranchDisabledReason({ + branchLocked: false, + hasRepo: false, + branchesLoading: false, + optionCount: 0, + }), + ).toBe("Select a repository first."); + }); + + it("reports loading while branches are being fetched", () => { + expect( + computeBranchDisabledReason({ + branchLocked: false, + hasRepo: true, + branchesLoading: true, + optionCount: 0, + }), + ).toContain("Loading"); + }); + + it("reports no branches available when the list is empty", () => { + expect( + computeBranchDisabledReason({ + branchLocked: false, + hasRepo: true, + branchesLoading: false, + optionCount: 0, + }), + ).toContain("No branches"); + }); + + it("returns undefined when the chip should be enabled", () => { + expect( + computeBranchDisabledReason({ + branchLocked: false, + hasRepo: true, + branchesLoading: false, + optionCount: 5, + }), + ).toBeUndefined(); + }); +}); diff --git a/apps/web/components/task-create-dialog-branch-utils.ts b/apps/web/components/task-create-dialog-branch-utils.ts new file mode 100644 index 0000000000..6ff2b36df7 --- /dev/null +++ b/apps/web/components/task-create-dialog-branch-utils.ts @@ -0,0 +1,82 @@ +/** + * Pure helpers for the branch chip in the task-create dialog. + * + * Extracted from `task-create-dialog-repo-chips.tsx` to keep that file under + * the 600-line cap. No React deps — these are string functions tested + * directly in `task-create-dialog-branch-utils.test.ts`. + */ + +/** + * Decide the muted text shown before the branch value to qualify intent. + * "current: " — local exec, picked branch matches the current checkout + * (also covers detached HEAD where the "branch" is the + * short SHA returned by the backend). + * "will switch to: " — local exec, picked branch differs from current. + * Surfaces the destructive `git checkout` decision the + * backend will make on submit. + * "from: " — fork mode (forking a new branch off this base) or + * non-local executors (worktree base). + * + * Returns "" when there's no value yet, so the chip's placeholder ("branch") + * doesn't get a misleading prefix. + */ +export function computeBranchPrefix({ + isLocalExecutor, + rowBranch, + currentLocalBranch, + freshBranchEnabled, +}: { + isLocalExecutor: boolean; + rowBranch: string; + currentLocalBranch: string; + freshBranchEnabled: boolean; +}): string { + if (!rowBranch) return ""; + if (freshBranchEnabled) return "from: "; + if (isLocalExecutor) { + if (currentLocalBranch && rowBranch === currentLocalBranch) return "current: "; + return "will switch to: "; + } + return "from: "; +} + +/** + * Branch chip hover tooltip, picked to match whatever prefix the chip is + * currently showing so the explanation lines up with the qualifier text. + * "current: " → no git ops will run; agent uses your existing checkout + * "will switch to: " → backend runs `git checkout` before the agent starts + * "from: " → a new branch / worktree is forked off this base + * "" → no branch picked yet; generic hint + */ +export function computeBranchTooltip(branchPrefix: string | undefined): string { + if (branchPrefix === "current: ") { + return "Your repository's current checkout. The agent runs against it as-is, no git operations."; + } + if (branchPrefix === "will switch to: ") { + return "The backend will run `git checkout` to switch your repository to this branch before the agent starts."; + } + if (branchPrefix === "from: ") { + return "Base branch. A new branch (or worktree) is forked from here and the agent runs there."; + } + return "Branch the agent will run against."; +} + +export function computeBranchDisabledReason({ + branchLocked, + hasRepo, + branchesLoading, + optionCount, +}: { + branchLocked: boolean; + hasRepo: boolean; + branchesLoading: boolean; + optionCount: number; +}): string | undefined { + if (branchLocked) { + return "The local executor uses your repository's current checkout, so the branch can't change here. Toggle 'Fork a new branch' to pick a different base."; + } + if (!hasRepo) return "Select a repository first."; + if (branchesLoading) return "Loading branches…"; + if (optionCount === 0) return "No branches available for this repository."; + return undefined; +} diff --git a/apps/web/components/task-create-dialog-handlers.ts b/apps/web/components/task-create-dialog-handlers.ts index 4716c08efd..942892d349 100644 --- a/apps/web/components/task-create-dialog-handlers.ts +++ b/apps/web/components/task-create-dialog-handlers.ts @@ -19,6 +19,13 @@ import { STORAGE_KEYS } from "@/lib/settings/constants"; function clearFreshBranch(fs: DialogFormState) { fs.setFreshBranchEnabled(false); fs.setCurrentLocalBranch(""); + // Set loading=true synchronously alongside the clear so the chip's + // autoselect effect (which runs bottom-up before useCurrentLocalBranchEffect + // can re-fire and set loading itself) sees the gate and skips. Otherwise + // the autoselect lands a last-used / preferred-default branch in row.branch + // before currentLocalBranch resolves, then the prefix logic computes + // "will switch to: master" instead of "current: master". + fs.setCurrentLocalBranchLoading(true); } function useRepositoryHandlers(fs: DialogFormState, repositories: Repository[]) { diff --git a/apps/web/components/task-create-dialog-pill.tsx b/apps/web/components/task-create-dialog-pill.tsx index 186bc2ed4e..4e4036264c 100644 --- a/apps/web/components/task-create-dialog-pill.tsx +++ b/apps/web/components/task-create-dialog-pill.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { cn } from "@/lib/utils"; import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; @@ -133,7 +133,39 @@ export function Pill({ tooltip, prefix, }: PillProps) { - const [open, setOpen] = useState(false); + const [open, setOpenState] = useState(false); + // Tracks the brief window after the popover closes during which the cursor + // is still hovering the trigger. Without this, Radix's Tooltip becomes + // uncontrolled the moment `open` flips to false and pops back open from the + // lingering hover. Holding `open={false}` for ~200ms lets the user move + // away naturally and prevents the tooltip from racing back in. + const [suppressTooltip, setSuppressTooltip] = useState(false); + const suppressTimerRef = useRef | null>(null); + const setOpen = useCallback((next: boolean) => { + setOpenState((prev) => { + if (prev && !next) { + if (suppressTimerRef.current) clearTimeout(suppressTimerRef.current); + setSuppressTooltip(true); + suppressTimerRef.current = setTimeout(() => { + setSuppressTooltip(false); + suppressTimerRef.current = null; + }, 200); + } + return next; + }); + }, []); + // Clear the suppression timer if the component unmounts while it's still + // ticking, so the eventual `setSuppressTooltip(false)` doesn't fire on a + // dead component. + useEffect( + () => () => { + if (suppressTimerRef.current) { + clearTimeout(suppressTimerRef.current); + suppressTimerRef.current = null; + } + }, + [], + ); const hasValue = !!value; // Prefix only renders alongside a real value; an empty chip with placeholder // ("branch") doesn't need "current: " in front of it. @@ -193,9 +225,13 @@ export function Pill({ if (!tooltip) return popover; - // Suppress the hover tooltip while the popover is open so they don't stack. + // Suppress the hover tooltip while the popover is open so they don't stack, + // and for a brief window after it closes so a lingering hover doesn't pop + // the tooltip back in (which Radix would otherwise do the moment we flip + // from controlled-closed back to uncontrolled). + const tooltipOpen = open || suppressTooltip ? false : undefined; return ( - + {popover} {tooltip} diff --git a/apps/web/components/task-create-dialog-repo-chips.test.tsx b/apps/web/components/task-create-dialog-repo-chips.test.tsx index cf207b4e65..9adb0b9c01 100644 --- a/apps/web/components/task-create-dialog-repo-chips.test.tsx +++ b/apps/web/components/task-create-dialog-repo-chips.test.tsx @@ -1,5 +1,4 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { computeBranchPrefix } from "./task-create-dialog-repo-chips"; import { render, screen, fireEvent, cleanup } from "@testing-library/react"; import type { Branch, Repository } from "@/lib/types/http"; import type { DialogFormState, TaskRepoRow } from "./task-create-dialog-types"; @@ -374,62 +373,3 @@ describe("RepoChipsRow", () => { mockBranches.value = { branches: [], isLoading: false }; }); }); - -describe("computeBranchPrefix", () => { - it("returns empty when no branch is picked yet", () => { - expect( - computeBranchPrefix({ isLocalExecutor: true, rowBranch: "", currentLocalBranch: "main" }), - ).toBe(""); - }); - - it("returns 'current: ' for local executor when row branch matches workspace current", () => { - expect( - computeBranchPrefix({ - isLocalExecutor: true, - rowBranch: "main", - currentLocalBranch: "main", - }), - ).toBe("current: "); - }); - - it("returns 'will switch to: ' for local executor when row branch differs from current", () => { - expect( - computeBranchPrefix({ - isLocalExecutor: true, - rowBranch: "develop", - currentLocalBranch: "main", - }), - ).toBe("will switch to: "); - }); - - it("treats unknown current branch as 'will switch to' on local executor", () => { - // Detached HEAD or local-status fetch error returns "" for currentLocalBranch. - // Without a known baseline we can't claim the picked branch is "current", - // so fall through to the destructive label — the user picked something - // explicit and the backend will checkout to it. - expect( - computeBranchPrefix({ - isLocalExecutor: true, - rowBranch: "main", - currentLocalBranch: "", - }), - ).toBe("will switch to: "); - }); - - it("returns 'from: ' for non-local executors regardless of current branch", () => { - expect( - computeBranchPrefix({ - isLocalExecutor: false, - rowBranch: "main", - currentLocalBranch: "main", - }), - ).toBe("from: "); - expect( - computeBranchPrefix({ - isLocalExecutor: false, - rowBranch: "develop", - currentLocalBranch: "", - }), - ).toBe("from: "); - }); -}); diff --git a/apps/web/components/task-create-dialog-repo-chips.tsx b/apps/web/components/task-create-dialog-repo-chips.tsx index 7239e71389..e72c7d05e5 100644 --- a/apps/web/components/task-create-dialog-repo-chips.tsx +++ b/apps/web/components/task-create-dialog-repo-chips.tsx @@ -17,6 +17,38 @@ import { type PillOption, } from "@/components/task-create-dialog-pill"; import { GitHubUrlSection } from "@/components/task-create-dialog-github-url"; +import { + computeBranchPrefix, + computeBranchTooltip, + computeBranchDisabledReason, +} from "@/components/task-create-dialog-branch-utils"; + +function runAutoselect({ + branches, + preferredDefaultBranch, + preferredDefaultBranchLoading, + onBranchChange, +}: { + branches: Array<{ name: string; type: "local" | "remote"; remote?: string }>; + preferredDefaultBranch: string | undefined; + preferredDefaultBranchLoading: boolean; + onBranchChange: (value: string) => void; +}) { + if (preferredDefaultBranchLoading) return; + // Local-executor flow: preferredDefaultBranch is the workspace's current + // ref (branch name OR detached-HEAD short SHA). Seed row.branch with it + // unconditionally so the chip displays "current: ". When the ref is a + // SHA (detached HEAD) it won't appear in the branches list, but falling + // through to autoSelectBranch would pick main/master and surface "will + // switch to: master", which contradicts what the user actually has checked + // out. Sending the SHA back on submit is a no-op because the backend's + // skip-when-equal check compares against the same SHA. + if (preferredDefaultBranch) { + onBranchChange(preferredDefaultBranch); + return; + } + autoSelectBranch(branches, onBranchChange); +} /** * Chip row for the task-create dialog. Renders one chip per row in @@ -197,6 +229,7 @@ function ChipsList({ isLocalExecutor, rowBranch: row.branch, currentLocalBranch: fs.currentLocalBranch, + freshBranchEnabled: !!fs.freshBranchEnabled, })} onRepositoryChange={(value) => onRowRepositoryChange(row.key, value)} onBranchChange={(value) => onRowBranchChange(row.key, value)} @@ -245,7 +278,11 @@ function FreshBranchToggle({ onClick={() => onToggle(!enabled)} data-testid="fresh-branch-toggle" aria-pressed={enabled} - aria-label={enabled ? "Fork a new branch" : "Use current branch"} + aria-label={ + enabled + ? "Fork a new branch from a base (turn off to use current checkout)" + : "Fork a new branch from a base instead of using current checkout" + } className={cn( "inline-flex h-7 w-7 items-center justify-center rounded-md border border-input cursor-pointer transition-colors", enabled @@ -256,10 +293,10 @@ function FreshBranchToggle({ - + {enabled - ? "Forking a new branch from the selected base. Click to use the existing branch instead." - : "Use the existing branch. Click to fork a new branch from a base instead."} + ? "Fork mode: a new branch will be created from the selected base before the agent runs. Click to turn off and use your repository's current checkout instead." + : "By default the local executor uses your repository's current checkout. Click to fork a new branch from a base instead, leaving your working tree untouched."} ); @@ -298,12 +335,15 @@ type RepoChipProps = { */ branchLocked?: boolean; /** - * When set, prefer this branch as the auto-selected default for an empty - * row.branch. Used by the local-executor flow to surface the workspace's - * current branch as the chip's default value (so submit always carries - * an explicit branch and the user sees what's on disk). Falls back to the - * existing last-used / preferred-default autoselect when unset or when - * the value isn't in the loaded branch list. + * When set, seed row.branch with this value (for an empty row). Used by + * the local-executor flow to surface the workspace's current ref — either + * a branch name like "main" or, on detached HEAD, the short commit SHA + * returned by the backend. The chip displays it verbatim ("current: main" + * or "current: 4fbc5d7"); on submit the backend's skip-when-equal check + * matches the same SHA so it's a no-op. + * + * When unset, the chip falls back to the existing last-used / preferred- + * default autoselect (main / master / develop, etc.). */ preferredDefaultBranch?: string; /** @@ -403,18 +443,12 @@ function useRepoChipData({ // so the current-branch preference gets first crack at row.branch. useEffect(() => { if (!branchSource || branchesLoading || branches.length === 0 || row.branch) return; - if (preferredDefaultBranchLoading) return; - if ( - preferredDefaultBranch && - branches.some((b) => { - const displayName = b.type === "remote" && b.remote ? `${b.remote}/${b.name}` : b.name; - return displayName === preferredDefaultBranch || b.name === preferredDefaultBranch; - }) - ) { - onBranchChange(preferredDefaultBranch); - return; - } - autoSelectBranch(branches, onBranchChange); + runAutoselect({ + branches, + preferredDefaultBranch, + preferredDefaultBranchLoading: !!preferredDefaultBranchLoading, + onBranchChange, + }); }, [ branchSource, branchesLoading, @@ -531,7 +565,7 @@ function RepoChip({ searchPlaceholder="Search branches..." emptyMessage="No branches" testId="branch-chip-trigger" - tooltip="Base branch" + tooltip={computeBranchTooltip(branchPrefix)} onRefresh={refreshBranches} refreshing={branchesLoading} filter={scoreBranch} @@ -555,54 +589,6 @@ function RepoChip({ ); } -/** - * Decide the muted text shown before the branch value to qualify intent. - * Local executor: "current: " when the user kept the workspace's current - * branch, "will switch to: " when they picked something different — surfaces - * the destructive `git checkout` decision the backend will make on submit. - * Non-local executors: "from: " — the picked branch is the base for the new - * worktree branch, not a switch target. - * - * Returns "" when there's no value yet, so the chip's placeholder ("branch") - * doesn't get a misleading prefix. - */ -export function computeBranchPrefix({ - isLocalExecutor, - rowBranch, - currentLocalBranch, -}: { - isLocalExecutor: boolean; - rowBranch: string; - currentLocalBranch: string; -}): string { - if (!rowBranch) return ""; - if (isLocalExecutor) { - if (currentLocalBranch && rowBranch === currentLocalBranch) return "current: "; - return "will switch to: "; - } - return "from: "; -} - -function computeBranchDisabledReason({ - branchLocked, - hasRepo, - branchesLoading, - optionCount, -}: { - branchLocked: boolean; - hasRepo: boolean; - branchesLoading: boolean; - optionCount: number; -}): string | undefined { - if (branchLocked) { - return "The local executor uses your repository's current checkout, so the branch can't change here. Toggle 'Fork a new branch' to pick a different base."; - } - if (!hasRepo) return "Select a repository first."; - if (branchesLoading) return "Loading branches…"; - if (optionCount === 0) return "No branches available for this repository."; - return undefined; -} - function normalizeRepoPath(path: string): string { return path.replace(/\\/g, "/").replace(/\/+$/g, ""); }