From bbb97f7e66b72c1d602ff58c9c0e39437cc51954 Mon Sep 17 00:00:00 2001 From: Iuliia Ivashko Date: Fri, 4 Sep 2026 02:10:22 +0300 Subject: [PATCH] fix(ui): keep New Worktree form state when worktree topology changes The reset-on-open effect also depended on generateUniqueSlug, which is derived from the available-worktree list, so a worktree appearing or disappearing while the dialog was open wiped the form (branch name, linked issue). Initialize once per open via a ref guard, pinned by a behavior test. Also tightens CreateWorktreeArgs/GitHub selection typing in place of type assertions. --- .../session/GitHubIntegrationDialog.tsx | 10 +- .../NewWorktreeDialog.behavior.test.tsx | 215 ++++++++++++++++++ .../components/session/NewWorktreeDialog.tsx | 65 +++--- 3 files changed, 258 insertions(+), 32 deletions(-) create mode 100644 packages/ui/src/components/session/NewWorktreeDialog.behavior.test.tsx diff --git a/packages/ui/src/components/session/GitHubIntegrationDialog.tsx b/packages/ui/src/components/session/GitHubIntegrationDialog.tsx index 966ff6ea..03bf5a3a 100644 --- a/packages/ui/src/components/session/GitHubIntegrationDialog.tsx +++ b/packages/ui/src/components/session/GitHubIntegrationDialog.tsx @@ -29,14 +29,14 @@ import { useI18n } from '@/lib/i18n'; type GitHubTab = 'issues' | 'prs'; +export type GitHubWorktreeSelection = + | { type: 'issue'; item: GitHubIssue; includeDiff?: boolean } + | { type: 'pr'; item: GitHubPullRequestSummary; includeDiff?: boolean }; + interface GitHubIntegrationDialogProps { open: boolean; onOpenChange: (open: boolean) => void; - onSelect: (result: { - type: 'issue' | 'pr'; - item: GitHubIssue | GitHubPullRequestSummary; - includeDiff?: boolean; - } | null) => void; + onSelect: (result: GitHubWorktreeSelection | null) => void; } interface ValidationResult { diff --git a/packages/ui/src/components/session/NewWorktreeDialog.behavior.test.tsx b/packages/ui/src/components/session/NewWorktreeDialog.behavior.test.tsx new file mode 100644 index 00000000..a638032b --- /dev/null +++ b/packages/ui/src/components/session/NewWorktreeDialog.behavior.test.tsx @@ -0,0 +1,215 @@ +import React, { act } from 'react'; +import { describe, expect, mock, test } from 'bun:test'; +import { createRoot } from 'react-dom/client'; +import { Window } from 'happy-dom'; +import { create } from 'zustand'; + +type GitHubSelection = { + type: 'issue'; + item: { number: number; title: string }; +}; + +type WorktreeState = { + availableWorktreesByProject: Map>; +}; + +const project = { id: 'project-a', path: '/workspace/project-a' }; +const useWorktreeStore = create(() => ({ + availableWorktreesByProject: new Map(), +})); +let selectGitHubItem: ((selection: GitHubSelection) => void) | null = null; + +const projectStoreState = { getActiveProject: () => project }; +const githubAuthState = { status: { connected: true }, hasChecked: true }; +const linearAuthState = { status: null, hasChecked: true }; +const uiState = { isMobile: false }; +const gitState = { fetchBranches: async () => undefined }; + +const selectProjectState = (selector: (state: typeof projectStoreState) => T): T => selector(projectStoreState); +const selectGitHubAuthState = (selector: (state: typeof githubAuthState) => T): T => selector(githubAuthState); +const selectLinearAuthState = (selector: (state: typeof linearAuthState) => T): T => selector(linearAuthState); +const selectUIState = (selector: (state: typeof uiState) => T): T => selector(uiState); +const selectGitState = (selector: (state: typeof gitState) => T): T => selector(gitState); + +const passthrough = ({ children }: React.PropsWithChildren) =>
{children}
; + +mock.module('@/components/ui/dialog', () => ({ + Dialog: ({ children, open }: React.PropsWithChildren<{ open: boolean }>) => open ? <>{children} : null, + DialogContent: passthrough, + DialogHeader: passthrough, + DialogTitle: passthrough, + DialogFooter: passthrough, +})); + +mock.module('@/components/ui/input', () => ({ + Input: (props: React.InputHTMLAttributes) => , +})); + +mock.module('@/components/ui/button', () => ({ + Button: ({ children, ...props }: React.ButtonHTMLAttributes) => ( + + ), +})); + +mock.module('@/components/ui', () => ({ + toast: { error: () => undefined, success: () => undefined }, +})); + +mock.module('@/components/ui/dropdown-menu', () => ({ + DropdownMenu: passthrough, + DropdownMenuContent: passthrough, + DropdownMenuTrigger: passthrough, +})); + +mock.module('@/components/ui/command', () => ({ + Command: passthrough, + CommandEmpty: passthrough, + CommandGroup: passthrough, + CommandInput: () => null, + CommandItem: passthrough, + CommandList: passthrough, + CommandSeparator: () => null, +})); + +mock.module('@/components/ui/sortable-tabs-strip', () => ({ SortableTabsStrip: () => null })); +mock.module('@/components/ui/MobileOverlayPanel', () => ({ MobileOverlayPanel: passthrough })); +mock.module('@/components/icon/Icon', () => ({ Icon: () => null })); +mock.module('@/components/ui/dropdown-trigger', () => ({ dropdownTriggerVariants: () => '' })); +mock.module('@/lib/utils', () => ({ cn: (...values: Array) => values.filter(Boolean).join(' ') })); + +mock.module('@/stores/useProjectsStore', () => ({ + useProjectsStore: selectProjectState, +})); +mock.module('@/stores/useGitHubAuthStore', () => ({ + useGitHubAuthStore: selectGitHubAuthState, +})); +mock.module('@/stores/useLinearAuthStore', () => ({ + useLinearAuthStore: selectLinearAuthState, +})); +mock.module('@/stores/useUIStore', () => ({ + useUIStore: selectUIState, +})); +mock.module('@/sync/session-ui-store', () => ({ + materializeOpenDraftSession: async () => null, + useSessionUIStore: useWorktreeStore, +})); +mock.module('@/sync/session-actions', () => ({ + createSession: async () => null, + updateSessionTitle: async () => undefined, +})); +mock.module('@/hooks/useRuntimeAPIs', () => ({ + useRuntimeAPIs: () => ({ github: {}, git: null, linear: null }), +})); +mock.module('@/stores/useGitStore', () => ({ + useGitBranches: () => ({ all: ['main'] }), + useGitLoadingBranches: () => false, + useGitStore: selectGitState, +})); +mock.module('@/lib/worktrees/worktreeManager', () => ({ + validateWorktreeCreate: async () => ({ ok: true, errors: [] }), +})); +mock.module('@/lib/worktrees/worktreeCreate', () => ({ createWorktreeWithDefaults: async () => null })); +mock.module('@/lib/worktrees/worktreeBootstrap', () => ({ waitForWorktreeBootstrap: async () => undefined })); +mock.module('@/lib/openchamberConfig', () => ({ + getWorktreeSetupCommands: async () => [], + getWorktreeSetupWaitEnabled: async () => false, +})); +mock.module('@/lib/worktrees/worktreeStatus', () => ({ getRootBranch: async () => 'main' })); +mock.module('@/lib/git/branchNameGenerator', () => ({ generateBranchSlug: () => 'draft-name' })); + +mock.module('./GitHubIntegrationDialog', () => ({ + GitHubIntegrationDialog: ({ onSelect }: { onSelect: (selection: GitHubSelection) => void }) => { + selectGitHubItem = onSelect; + return null; + }, +})); +mock.module('./LinearIssuePickerDialog', () => ({ LinearIssuePickerDialog: () => null })); + +const { NewWorktreeDialog } = await import('./NewWorktreeDialog'); +const { I18nProvider } = await import('@/lib/i18n'); + +const DOM_GLOBAL_NAMES = [ + 'window', + 'document', + 'navigator', + 'Node', + 'Element', + 'HTMLElement', + 'HTMLIFrameElement', + 'localStorage', + 'IS_REACT_ACT_ENVIRONMENT', +] as const; + +const installDom = () => { + const happyWindow = new Window({ url: 'http://localhost' }); + const previous = DOM_GLOBAL_NAMES.map( + (name) => [name, Object.getOwnPropertyDescriptor(globalThis, name)] as const, + ); + const values = { + window: happyWindow, + document: happyWindow.document, + navigator: happyWindow.navigator, + Node: happyWindow.Node, + Element: happyWindow.Element, + HTMLElement: happyWindow.HTMLElement, + HTMLIFrameElement: happyWindow.HTMLIFrameElement, + localStorage: happyWindow.localStorage, + IS_REACT_ACT_ENVIRONMENT: true, + }; + for (const name of DOM_GLOBAL_NAMES) { + Object.defineProperty(globalThis, name, { value: values[name], configurable: true, writable: true }); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + return { + container, + restore: () => { + for (const [name, descriptor] of previous) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor); + else Reflect.deleteProperty(globalThis, name); + } + }, + }; +}; + +describe('NewWorktreeDialog behavior', () => { + test('preserves selected issue values when available worktree names change', async () => { + const dom = installDom(); + const root = createRoot(dom.container); + useWorktreeStore.setState({ availableWorktreesByProject: new Map() }); + + try { + await act(async () => root.render( + + undefined} /> + , + )); + if (!selectGitHubItem) throw new Error('Expected GitHub selection handler'); + + await act(async () => selectGitHubItem?.({ + type: 'issue', + item: { number: 42, title: 'Keep the selected issue' }, + })); + + const [branchInput, worktreeInput] = dom.container.querySelectorAll('input'); + expect(branchInput?.value).toBe('issue-42-draft-name'); + expect(worktreeInput?.value).toBe('issue-42-draft-name'); + expect(dom.container.textContent).toContain('Keep the selected issue'); + + await act(async () => useWorktreeStore.setState({ + availableWorktreesByProject: new Map([ + [project.path, [{ name: 'newly-created-worktree' }]], + ]), + })); + + expect(branchInput?.value).toBe('issue-42-draft-name'); + expect(worktreeInput?.value).toBe('issue-42-draft-name'); + expect(dom.container.textContent).toContain('Keep the selected issue'); + } finally { + await act(async () => root.unmount()); + selectGitHubItem = null; + dom.restore(); + } + }); +}); diff --git a/packages/ui/src/components/session/NewWorktreeDialog.tsx b/packages/ui/src/components/session/NewWorktreeDialog.tsx index f427e827..425e9281 100644 --- a/packages/ui/src/components/session/NewWorktreeDialog.tsx +++ b/packages/ui/src/components/session/NewWorktreeDialog.tsx @@ -51,7 +51,7 @@ import { } from '@/lib/worktrees/worktreeSourceBranchPreference'; import { useRuntimeAPIs } from '@/hooks/useRuntimeAPIs'; import { useGitBranches, useGitStore, useGitLoadingBranches } from '@/stores/useGitStore'; -import { GitHubIntegrationDialog } from './GitHubIntegrationDialog'; +import { GitHubIntegrationDialog, type GitHubWorktreeSelection } from './GitHubIntegrationDialog'; import { LinearIssuePickerDialog } from './LinearIssuePickerDialog'; import { SortableTabsStrip } from '@/components/ui/sortable-tabs-strip'; import { MobileOverlayPanel } from '@/components/ui/MobileOverlayPanel'; @@ -65,7 +65,7 @@ import type { LinearIssue, LinearIssueComment, } from '@/lib/api/types'; -import type { ProjectRef } from '@/lib/worktrees/worktreeManager'; +import type { CreateWorktreeArgs, ProjectRef } from '@/lib/worktrees/worktreeManager'; import { useI18n } from '@/lib/i18n'; type Mode = 'new-branch' | 'existing-branch'; @@ -456,6 +456,7 @@ export function NewWorktreeDialog({ // Creation state const [isCreating, setIsCreating] = React.useState(false); const [validationAbortController, setValidationAbortController] = React.useState(null); + const initializedForCurrentOpen = React.useRef(false); const resolveDefaultAgentName = React.useCallback((): string | undefined => { const configState = useConfigStore.getState(); @@ -493,9 +494,7 @@ export function NewWorktreeDialog({ : undefined; const provider = configState.providers.find((p) => p.id === providerID); - const model = provider?.models.find((m: Record) => (m as { id?: string }).id === modelID) as - | { variants?: Record } - | undefined; + const model = provider?.models.find((m) => m.id === modelID); const variants = model?.variants; if (!variants) return settingsDefaultVariant || currentVariant || undefined; if (settingsDefaultVariant && Object.prototype.hasOwnProperty.call(variants, settingsDefaultVariant)) return settingsDefaultVariant; @@ -763,7 +762,12 @@ export function NewWorktreeDialog({ // Reset state on each open. Resetting on close would empty the form during // the close animation, causing visible flicker. React.useLayoutEffect(() => { - if (!open) return; + if (!open) { + initializedForCurrentOpen.current = false; + return; + } + if (initializedForCurrentOpen.current) return; + initializedForCurrentOpen.current = true; setMode('new-branch'); setExistingBranchState({ @@ -840,14 +844,15 @@ export function NewWorktreeDialog({ if (normalizedBranch && normalizedWorktree) { const linkedPr = mode === 'new-branch' ? newBranchState.linkedPr : null; const prConfig = linkedPr ? resolvePrWorktreeConfig(linkedPr, localBranches, remoteBranches) : null; - const result = await validateWorktreeCreate(projectRef, { + const validateArgs: CreateWorktreeArgs = { mode: mode === 'existing-branch' || prConfig ? 'existing' : 'new', branchName: normalizedBranch, worktreeName: normalizedWorktree, existingBranch: prConfig?.existingBranch ?? (mode === 'existing-branch' ? normalizedBranch : undefined), - ...(prConfig?.ensureRemoteName ? { ensureRemoteName: prConfig.ensureRemoteName } : {}), - ...(prConfig?.ensureRemoteUrl ? { ensureRemoteUrl: prConfig.ensureRemoteUrl } : {}), - }); + }; + if (prConfig?.ensureRemoteName) validateArgs.ensureRemoteName = prConfig.ensureRemoteName; + if (prConfig?.ensureRemoteUrl) validateArgs.ensureRemoteUrl = prConfig.ensureRemoteUrl; + const result = await validateWorktreeCreate(projectRef, validateArgs); if (abortController.signal.aborted) return; @@ -961,13 +966,13 @@ export function NewWorktreeDialog({ const sourceBranch = newBranchState.sourceBranch; let sourceLabel = ''; - const args = (() => { + const args: CreateWorktreeArgs = (() => { if (linkedPr) { const prConfig = resolvePrWorktreeConfig(linkedPr, localBranches, remoteBranches); sourceLabel = prConfig.sourceLabel; - return { + const prArgs: CreateWorktreeArgs = { preferredName: normalizedBranch || normalizedWorktree, - mode: 'existing' as const, + mode: 'existing', branchName: normalizedBranch, worktreeName: normalizedWorktree, existingBranch: prConfig.existingBranch, @@ -976,22 +981,24 @@ export function NewWorktreeDialog({ upstreamRemote: prConfig.upstreamRemote, upstreamBranch: prConfig.upstreamBranch, returnAfterDirectoryCreated: true, - ...(prConfig.ensureRemoteName ? { ensureRemoteName: prConfig.ensureRemoteName } : {}), - ...(prConfig.ensureRemoteUrl ? { ensureRemoteUrl: prConfig.ensureRemoteUrl } : {}), }; + if (prConfig.ensureRemoteName) prArgs.ensureRemoteName = prConfig.ensureRemoteName; + if (prConfig.ensureRemoteUrl) prArgs.ensureRemoteUrl = prConfig.ensureRemoteUrl; + return prArgs; } sourceLabel = mode === 'new-branch' ? sourceBranch : ''; - return { + const baseArgs: CreateWorktreeArgs = { preferredName: normalizedBranch || normalizedWorktree, - mode: mode === 'existing-branch' ? 'existing' as const : 'new' as const, + mode: mode === 'existing-branch' ? 'existing' : 'new', branchName: mode === 'existing-branch' ? undefined : normalizedBranch, worktreeName: normalizedWorktree, existingBranch: mode === 'existing-branch' ? normalizedBranch : undefined, setupCommands, returnAfterDirectoryCreated: true, - ...(sourceBranch && mode === 'new-branch' ? { startRef: sourceBranch } : {}), }; + if (sourceBranch && mode === 'new-branch') baseArgs.startRef = sourceBranch; + return baseArgs; })(); const metadata = await createWorktreeWithDefaults(projectRef, args); @@ -1084,11 +1091,7 @@ export function NewWorktreeDialog({ }; // Handle GitHub selection - const handleGitHubSelect = (result: { - type: 'issue' | 'pr'; - item: GitHubIssue | GitHubPullRequestSummary; - includeDiff?: boolean; - } | null) => { + const handleGitHubSelect = (result: GitHubWorktreeSelection | null) => { if (!result) { setNewBranchState(prev => ({ ...prev, @@ -1102,7 +1105,7 @@ export function NewWorktreeDialog({ } if (result.type === 'issue') { - const issue = result.item as GitHubIssue; + const issue = result.item; const newBranchName = `issue-${issue.number}-${generateBranchSlug()}`; setNewBranchState(prev => ({ ...prev, @@ -1115,7 +1118,7 @@ export function NewWorktreeDialog({ isSyncingWorktreeName: true, })); } else if (result.type === 'pr') { - const pr = result.item as GitHubPullRequestSummary; + const pr = result.item; setNewBranchState(prev => ({ ...prev, linkedPr: pr, @@ -1261,7 +1264,11 @@ export function NewWorktreeDialog({ { id: 'existing-branch', label: t('session.newWorktree.mode.existingBranch'), icon: }, ]} activeId={mode} - onSelect={(id) => handleModeChange(id as Mode)} + onSelect={(id) => { + if (id === 'new-branch' || id === 'existing-branch') { + handleModeChange(id); + } + }} variant="active-pill" layoutMode="fit" className="w-full" @@ -1767,7 +1774,11 @@ export function NewWorktreeDialog({ { id: 'existing-branch', label: t('session.newWorktree.mode.existingBranch'), icon: }, ]} activeId={mode} - onSelect={(id) => handleModeChange(id as Mode)} + onSelect={(id) => { + if (id === 'new-branch' || id === 'existing-branch') { + handleModeChange(id); + } + }} variant="active-pill" layoutMode="fit" className="w-full"