From 706ae5b6ffc8bd2d9b07e137664b83bf840d6d36 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Sat, 29 Aug 2026 10:40:10 +0800 Subject: [PATCH] fix(flow-chat): reorganize composer context controls --- .../dispatch/DispatchTargetPicker.scss | 3 +- .../dispatch/DispatchTargetPicker.test.tsx | 77 +++++++++++++++++ .../dispatch/DispatchTargetPicker.tsx | 68 ++++++++++++--- .../components/ChatInputWorkspaceStrip.scss | 19 +--- .../ChatInputWorkspaceStrip.test.tsx | 60 +++++++++++-- .../components/ChatInputWorkspaceStrip.tsx | 86 ++++++++++--------- .../ChatInputWorkspaceStripLayout.test.ts | 47 ++++++++-- src/web-ui/src/locales/en-US/worktrees.json | 1 + src/web-ui/src/locales/zh-CN/worktrees.json | 1 + src/web-ui/src/locales/zh-TW/worktrees.json | 1 + 10 files changed, 278 insertions(+), 85 deletions(-) diff --git a/src/web-ui/src/features/dispatch/DispatchTargetPicker.scss b/src/web-ui/src/features/dispatch/DispatchTargetPicker.scss index 3616c1e1fa..59014ecf2b 100644 --- a/src/web-ui/src/features/dispatch/DispatchTargetPicker.scss +++ b/src/web-ui/src/features/dispatch/DispatchTargetPicker.scss @@ -248,8 +248,7 @@ width: 18px; padding: 0; - > span, - > .dispatch-target-picker__chevron { + > span { display: none; } } diff --git a/src/web-ui/src/features/dispatch/DispatchTargetPicker.test.tsx b/src/web-ui/src/features/dispatch/DispatchTargetPicker.test.tsx index d34d8d2532..65ecaf8efb 100644 --- a/src/web-ui/src/features/dispatch/DispatchTargetPicker.test.tsx +++ b/src/web-ui/src/features/dispatch/DispatchTargetPicker.test.tsx @@ -97,6 +97,7 @@ describe('DispatchTargetPicker overlay', () => { const trigger = container.querySelector( '[data-testid="chat-input-dispatch-trigger"]', ); + expect(trigger?.querySelectorAll('svg')).toHaveLength(1); await act(async () => trigger?.click()); const menu = document.querySelector('[data-testid="dispatch-target-menu"]'); @@ -105,4 +106,80 @@ describe('DispatchTargetPicker overlay', () => { expect(menu?.style.left).toBe('240px'); expect(menu?.style.top).toBe('293px'); }); + + it('offers New Worktree inside the local target and reflects the selected local mode', async () => { + const onSelectLocal = vi.fn(); + const onWorktreeChange = vi.fn(); + const localWorktreeControl = { + enabled: false, + locked: false, + label: 'New Worktree', + description: 'Run in an isolated worktree.', + onChange: onWorktreeChange, + }; + + await act(async () => { + root.render( + , + ); + }); + + let trigger = container.querySelector( + '[data-testid="chat-input-dispatch-trigger"]', + ); + expect(trigger?.textContent).toBe('chatInput.dispatch.local'); + expect(trigger?.querySelectorAll('svg')).toHaveLength(1); + + await act(async () => trigger?.click()); + + const localOption = document.querySelector( + '[data-testid="dispatch-target-local-option"]', + ); + const worktreeOption = document.querySelector( + '[data-testid="dispatch-target-new-worktree-option"]', + ); + expect(localOption?.getAttribute('aria-checked')).toBe('true'); + expect(worktreeOption?.textContent).toContain('New Worktree'); + expect(worktreeOption?.getAttribute('aria-checked')).toBe('false'); + + await act(async () => worktreeOption?.click()); + expect(onSelectLocal).toHaveBeenCalledTimes(1); + expect(onWorktreeChange).toHaveBeenLastCalledWith(true); + expect(document.querySelector('[data-testid="dispatch-target-menu"]')).toBeNull(); + + await act(async () => { + root.render( + , + ); + }); + + trigger = container.querySelector( + '[data-testid="chat-input-dispatch-trigger"]', + ); + expect(trigger?.textContent).toBe('New Worktree'); + expect(trigger?.querySelectorAll('svg')).toHaveLength(1); + + await act(async () => trigger?.click()); + expect(document.querySelector( + '[data-testid="dispatch-target-new-worktree-option"]', + )?.getAttribute('aria-checked')).toBe('true'); + + await act(async () => document.querySelector( + '[data-testid="dispatch-target-local-option"]', + )?.click()); + expect(onSelectLocal).toHaveBeenCalledTimes(2); + expect(onWorktreeChange).toHaveBeenLastCalledWith(false); + }); }); diff --git a/src/web-ui/src/features/dispatch/DispatchTargetPicker.tsx b/src/web-ui/src/features/dispatch/DispatchTargetPicker.tsx index 6e0b195e93..7750a337bb 100644 --- a/src/web-ui/src/features/dispatch/DispatchTargetPicker.tsx +++ b/src/web-ui/src/features/dispatch/DispatchTargetPicker.tsx @@ -8,6 +8,7 @@ import React, { } from 'react'; import { createPortal } from 'react-dom'; import { + FolderGit2, Laptop, Loader2, MonitorSmartphone, @@ -36,6 +37,13 @@ interface DispatchTargetPickerProps { sourceWorkspacePath?: string; locked: boolean; disabled?: boolean; + localWorktreeControl?: { + enabled: boolean; + locked: boolean; + label: string; + description: string; + onChange: (enabled: boolean) => void; + }; onSelectLocal?: () => void; onSelectTarget: (selection: DispatchSelection) => void; } @@ -49,6 +57,7 @@ export const DispatchTargetPicker: React.FC = ({ sourceWorkspacePath, locked, disabled = false, + localWorktreeControl, onSelectLocal, onSelectTarget, }) => { @@ -72,8 +81,11 @@ export const DispatchTargetPicker: React.FC = ({ layoutRevision: `${targets.length}:${loading}:${error ?? ''}`, }); + const localDisplayLabel = localWorktreeControl?.enabled + ? localWorktreeControl.label + : t('chatInput.dispatch.local'); const displayLabel = target.kind === 'local' - ? t('chatInput.dispatch.local') + ? localDisplayLabel : target.displayName; const tooltip = locked ? t('chatInput.dispatch.locked', { target: displayLabel }) @@ -116,6 +128,22 @@ export const DispatchTargetPicker: React.FC = ({ [targets], ); + const selectLocalMode = (worktreeEnabled: boolean) => { + setOpen(false); + onSelectLocal?.(); + if ( + localWorktreeControl + && localWorktreeControl.enabled !== worktreeEnabled + ) { + localWorktreeControl.onChange(worktreeEnabled); + } + }; + + const localDirectorySelected = + target.kind === 'local' && !localWorktreeControl?.enabled; + const localWorktreeSelected = + target.kind === 'local' && !!localWorktreeControl?.enabled; + const menu = open ? ( = ({ + {localWorktreeControl ? ( + + ) : null}
@@ -312,14 +359,13 @@ export const DispatchTargetPicker: React.FC = ({ }} > {target.kind === 'local' - ? + ? localWorktreeControl?.enabled + ? + : : target.kind === 'device' ? : } {displayLabel} - {!locked ? ( - - ) : null} {menu && createPortal(menu, getAppearanceOverlayHost())} diff --git a/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.scss b/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.scss index 2fda44e57f..4cd28fd400 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.scss +++ b/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.scss @@ -79,9 +79,8 @@ $track-item-gap: 10px; opacity: 0.55; } - // One glyph size for the whole track, enforced here so a component that - // ships its own `size` prop (the dispatch picker's chevron) still lands on - // it without the strip reaching into that component. + // One glyph size for the whole track, enforced here so component-owned + // icons land on the same rhythm without the strip reaching into them. > svg { flex: none; width: 12px; @@ -139,15 +138,6 @@ $track-item-gap: 10px; padding-bottom: 1px; text-overflow: ellipsis; } - - // The chevron is punctuation, not one of the track's semantic glyphs. - // At the shared 12px it outweighed the mark that says which host this - // is, which is the thing worth reading. - .dispatch-target-picker__chevron { - width: 10px; - height: 10px; - opacity: 0.55; - } } } @@ -688,11 +678,6 @@ $track-item-gap: 10px; padding-bottom: 1px; } - &__workspace-chevron { - flex: none; - opacity: 0.55; - } - // Only the interactive form answers a click — the hover fill is the whole // difference between a fact and a control here. &__workspace--switchable { diff --git a/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.test.tsx b/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.test.tsx index a3f8b986c2..087d5e3c48 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.test.tsx +++ b/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.test.tsx @@ -32,6 +32,7 @@ vi.mock('react-i18next', () => ({ 'deepReviewConsent.strategyLabels.normal': 'Standard', 'reasoningSelector.auto': 'Auto', 'chatInput.permissionMode.ask.label': 'Ask', + 'strip.newWorktree': 'New Worktree', } as Record)[key] ?? options?.defaultValue ?? key, }), })); @@ -67,10 +68,38 @@ vi.mock('@/tools/git/hooks/useGitState', () => ({ })); // The real picker pulls in account state, SSH dialogs and a lazy remote-connect -// route. This suite only asserts whether the strip mounts it at all. +// route. This suite observes the strip-to-picker contract through a lightweight +// stand-in; picker behavior itself stays covered in its focused suite. vi.mock('@/features/dispatch/DispatchTargetPicker', () => ({ - DispatchTargetPicker: ({ locked }: { locked: boolean }) => ( -
+ DispatchTargetPicker: ({ + locked, + localWorktreeControl, + }: { + locked: boolean; + localWorktreeControl?: { + enabled: boolean; + locked: boolean; + label: string; + onChange: (enabled: boolean) => void; + }; + }) => ( +
+ {localWorktreeControl ? ( + + ) : null} +
), })); @@ -881,13 +910,14 @@ describe('ChatInputWorkspaceStrip git refresh behavior', () => { expect(container.querySelector('[data-testid="chat-input-worktree-toggle"]')).toBeNull(); }); - it('shows the dispatch picker and the worktree toggle together in a Git workspace', async () => { + it('orders workspace and branch before the target and nests worktree under local', async () => { + const onChange = vi.fn(); await act(async () => { root.render( { ); }); - expect(container.querySelector('[data-testid="chat-input-worktree-toggle"]')).not.toBeNull(); - expect(container.querySelector('[data-testid="chat-input-dispatch-trigger"]')).not.toBeNull(); + const context = container.querySelector('[data-bf-part="context"]'); + const location = context?.querySelector('.bitfun-chat-input-workspace-strip__location'); + const dispatchTrigger = context?.querySelector( + '[data-testid="chat-input-dispatch-trigger"]', + ); + expect(context).not.toBeNull(); + expect(location).not.toBeNull(); + expect(dispatchTrigger).not.toBeNull(); + expect(Array.from(context?.children ?? []).indexOf(location as Element)) + .toBeLessThan(Array.from(context?.children ?? []).indexOf(dispatchTrigger as Element)); + expect(container.querySelector('[data-testid="chat-input-worktree-toggle"]')).toBeNull(); + expect(dispatchTrigger?.dataset.worktreeEnabled).toBe('false'); + expect(dispatchTrigger?.dataset.worktreeLabel).toBe('New Worktree'); + + await act(async () => container.querySelector( + '[data-testid="dispatch-target-new-worktree-option"]', + )?.click()); + expect(onChange).toHaveBeenCalledWith(true); }); it('shows the dispatched branch instead of the source branch once dispatch is locked', async () => { diff --git a/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.tsx b/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.tsx index 3f2a01646e..d90dce1230 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.tsx +++ b/src/web-ui/src/flow_chat/components/ChatInputWorkspaceStrip.tsx @@ -1,9 +1,10 @@ /** * Two fixed rails in the composer's upper context band. * - * The left rail is the situation the session is in — where it runs, on which - * branch, and whether that branch is isolated in a worktree. The right rail is - * the contract for the next turn — how much confirmation it asks for and how + * The left rail is the situation the session is in — its workspace and branch, + * followed by the local/remote execution target. Worktree isolation is a local + * target mode. The right rail is the contract for the next turn — how much + * confirmation it asks for and how * much context is left. Nothing is centered and no column template is * conditional, so a control appearing or disappearing cannot move the rest of * the track. @@ -14,7 +15,6 @@ import { createPortal } from 'react-dom'; import { useTranslation } from 'react-i18next'; import { Check, - ChevronDown, Circle, EyeOff, GitBranch, @@ -95,7 +95,7 @@ export interface ChatInputWorkspaceStripProps { /** Resolved target bound to the active session. */ executionTarget?: SessionExecutionTarget; /** - * Per-session worktree isolation, rendered next to the branch for Git workspaces. + * Per-session worktree isolation, exposed as a local execution-target mode. * Omitted when the session cannot host a worktree at all (remote, no session). */ worktreeControl?: { @@ -402,18 +402,25 @@ export const ChatInputWorkspaceStrip: React.FC = ( const usageTooltip = `${formatCompactTokenCount(usageCurrentTokens)}/${formatCompactTokenCount(usageMaxTokens)} ${usagePercentage}%`; const usageDash = `${((usagePercentage / 100) * 62.83).toFixed(2)} 62.83`; - const handleWorktreeToggle = () => { + const handleWorktreeChange = (enabled: boolean) => { if (!worktreeControl || worktreeToggleDisabled) { return; } - const nextEnabled = !worktreeEnabledRef.current; - worktreeEnabledRef.current = nextEnabled; - worktreeControl.onChange(nextEnabled); + if (worktreeEnabledRef.current === enabled) { + return; + } + worktreeEnabledRef.current = enabled; + worktreeControl.onChange(enabled); + }; + + const handleWorktreeToggle = () => { + handleWorktreeChange(!worktreeEnabledRef.current); }; // The branch reports where the session sits; it is a fact, not a control. - // Isolation is the control, and it says so with a checkbox of its own rather - // than hiding a switch under a label that reads as a breadcrumb. + // Isolation is selected from the local destination menu when that picker is + // present; the checkbox below remains a fallback for embedded surfaces that + // do not expose dispatch targets. const renderBranchChip = () => ( @@ -460,11 +467,6 @@ export const ChatInputWorkspaceStrip: React.FC = ( }} > {label} - {workspaceMenuOpen ? createPortal( @@ -548,10 +550,9 @@ export const ChatInputWorkspaceStrip: React.FC = ( ) : null); - // A hairline between segments that are not part of the same thought — the - // host picker, the location phrase and the isolation switch are three - // separate statements and the row reads calmer when they part on a rule - // instead of running together. + // A hairline parts the workspace/branch coordinate from the execution + // destination. Worktree isolation belongs inside the local destination + // menu, so it no longer creates a third statement on this rail. const renderDivider = (key: string) => ( = ( data-bf-part="context" className="bitfun-chat-input-workspace-strip__context" > - {showDispatchPicker && dispatchControl ? ( - <> - - - ) : null} - {showDispatchPicker && label ? renderDivider('context-host') : null} {label ? ( - <> - - {renderWorkspaceControl()} - {renderBranchChip()} - - {showWorktreeToggle ? renderDivider('context-isolation') : null} - {renderWorktreeToggle()} - + + {renderWorkspaceControl()} + {renderBranchChip()} + + ) : null} + {showDispatchPicker && label ? renderDivider('context-target') : null} + {showDispatchPicker && dispatchControl ? ( + ) : null} + {!showDispatchPicker && showWorktreeToggle + ? renderDivider('context-isolation') + : null} + {!showDispatchPicker ? renderWorktreeToggle() : null}
readLocalFile('ChatInputWorkspaceStrip.scss'); const readChatInputStylesheet = () => readLocalFile('ChatInput.scss'); const readWorkspaceStripComponent = () => readLocalFile('ChatInputWorkspaceStrip.tsx'); +const readDispatchTargetPickerComponent = () => readFileSync( + fileURLToPath(new URL('../../features/dispatch/DispatchTargetPicker.tsx', import.meta.url)), + 'utf8', +).replace(/\r\n/g, '\n'); describe('composer context track layout', () => { it('is two fixed rails rather than a conditional column template', () => { @@ -158,14 +162,21 @@ describe('composer context track layout', () => { const stylesheet = readWorkspaceStripStylesheet(); const contextIndex = component.indexOf('data-bf-part="context"'); const nextIndex = component.indexOf('data-bf-part="next"'); + const contextMarkup = component.slice(contextIndex, nextIndex); + const locationIndex = contextMarkup.indexOf('__location'); + const targetIndex = contextMarkup.indexOf(' { expect(stylesheet).toMatch( /@media \(max-width: 460px\)[\s\S]*?__worktree-label \{\n {6}display: none;/, ); - // …while the isolation checkbox, the permission shield, and the context - // ring stay. + // …while the embedded-surface isolation fallback, the permission shield, + // and the context ring stay. expect(stylesheet).not.toMatch(/@media[\s\S]*?__worktree-toggle \{\n {6}display: none;/); expect(stylesheet).not.toMatch(/@media[\s\S]*?__usage-ring \{\n {6}display: none;/); expect(stylesheet).not.toMatch(/@media[\s\S]*?__permission-overview-icon \{\n {6}display: none;/); }); - it('states worktree isolation as a checkbox rather than hiding it on the branch', () => { + it('nests worktree isolation in the local target and keeps an embedded fallback', () => { const component = readWorkspaceStripComponent(); + const targetPicker = readDispatchTargetPickerComponent(); const stylesheet = readWorkspaceStripStylesheet(); - // The branch is a fact and the isolation is a control. Folding the switch - // into the branch label left the composer with no visible way to say the - // session can run somewhere else. + // The normal composer offers both local modes in one target menu. A rare + // embedded surface without that picker retains the standalone switch so + // it does not lose worktree capability. + expect(component).toContain('localWorktreeControl={showWorktreeToggle && worktreeControl ? {'); + expect(component).toContain('!showDispatchPicker ? renderWorktreeToggle() : null'); + expect(targetPicker).toContain('data-testid="dispatch-target-local-option"'); + expect(targetPicker).toContain('data-testid="dispatch-target-new-worktree-option"'); + expect(targetPicker).toContain('{localWorktreeControl.label}'); + expect(targetPicker).toContain('role="menuitemradio"'); expect(component).toContain('role="switch"'); expect(component).toContain('__worktree-toggle'); expect(component).not.toContain('__chip--branch-toggle'); @@ -462,6 +480,18 @@ describe('composer context track layout', () => { expect(stylesheet).not.toMatch(/&__worktree-toggle \{[\s\S]*?border: 1px/); }); + it('removes down chevrons from the clickable workspace and target labels', () => { + const component = readWorkspaceStripComponent(); + const targetPicker = readDispatchTargetPickerComponent(); + const stylesheet = readWorkspaceStripStylesheet(); + + expect(component).not.toContain('ChevronDown'); + expect(component).not.toContain('__workspace-chevron'); + expect(targetPicker).not.toContain('chevron-down'); + expect(targetPicker).not.toContain('__chevron'); + expect(stylesheet).not.toContain('__workspace-chevron'); + }); + it('answers a blocked turn from the composer stack, not from over the transcript', () => { const chatInput = readLocalFile('ChatInput.tsx'); const container = readLocalFile('modern/ModernFlowChatContainer.tsx'); @@ -523,5 +553,6 @@ describe('composer context track layout', () => { 'const dispatchPickerLocked = !!dispatchControl && (dispatchControl.locked || !isGitWorkspace);', ); expect(component).toContain('locked={dispatchPickerLocked}'); + expect(component).toContain('localWorktreeControl={showWorktreeToggle && worktreeControl ? {'); }); }); diff --git a/src/web-ui/src/locales/en-US/worktrees.json b/src/web-ui/src/locales/en-US/worktrees.json index 3c3dad1a1e..d7bce29cf7 100644 --- a/src/web-ui/src/locales/en-US/worktrees.json +++ b/src/web-ui/src/locales/en-US/worktrees.json @@ -4,6 +4,7 @@ }, "strip": { "toggleLabel": "worktree", + "newWorktree": "New Worktree", "toggleOffDescription": "Run this session in its own Git worktree so parallel agents do not disturb each other.", "toggleOnDescription": "Running in an isolated worktree at {{path}}. Turn off to run in the project directory.", "togglePendingOnDescription": "Worktree isolation is armed. The worktree will be created after you send the first message.", diff --git a/src/web-ui/src/locales/zh-CN/worktrees.json b/src/web-ui/src/locales/zh-CN/worktrees.json index 5ce0edf0e6..ef9bd49bd0 100644 --- a/src/web-ui/src/locales/zh-CN/worktrees.json +++ b/src/web-ui/src/locales/zh-CN/worktrees.json @@ -4,6 +4,7 @@ }, "strip": { "toggleLabel": "worktree", + "newWorktree": "New Worktree", "toggleOffDescription": "让这个会话在独立的 Git worktree 中执行，多个 agent 并行跑任务时互不打扰。", "toggleOnDescription": "正在隔离的 worktree 中执行：{{path}}。关闭后会回到项目目录。", "togglePendingOnDescription": "已开启 worktree 隔离；发送第一条消息后才会创建 worktree。", diff --git a/src/web-ui/src/locales/zh-TW/worktrees.json b/src/web-ui/src/locales/zh-TW/worktrees.json index 8c26ab04e3..e2ccf2a83a 100644 --- a/src/web-ui/src/locales/zh-TW/worktrees.json +++ b/src/web-ui/src/locales/zh-TW/worktrees.json @@ -4,6 +4,7 @@ }, "strip": { "toggleLabel": "worktree", + "newWorktree": "New Worktree", "toggleOffDescription": "讓這個工作階段在獨立的 Git worktree 中執行，多個 agent 並行執行任務時互不干擾。", "toggleOnDescription": "正在隔離的 worktree 中執行：{{path}}。關閉後會回到專案目錄。", "togglePendingOnDescription": "已開啟 worktree 隔離；傳送第一則訊息後才會建立 worktree。",