diff --git a/00-review-context.md b/00-review-context.md deleted file mode 100644 index 44aaa5d51ab..00000000000 --- a/00-review-context.md +++ /dev/null @@ -1,60 +0,0 @@ -# Review Context - -## Branch Info - -- Base: origin/main (`95526a42c4ab06518c7cd44577ff2ed270a0edf3`) -- Current: `Jinwoo-H/floating-terminal-improvements` - -## Changed Files Summary - -- M `src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx` -- M `src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx` -- M `src/renderer/src/store/slices/browser.test.ts` -- M `src/renderer/src/store/slices/browser.ts` -- M `src/renderer/src/store/slices/editor.test.ts` -- M `src/renderer/src/store/slices/editor.ts` - -## Changed Line Ranges (PR Scope) - - - -| File | Changed Lines | -| --- | --- | -| `src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx` | 1-3, 6-7, 18-22, 31-35, 37-41, 45-47, 63-65, 67, 72-75, 139-150, 157-204, 263-275, 277-288, 296-300, 303-305, 307, 309-312, 316, 404-405, 497-499 | -| `src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx` | 1-7, 9, 12-22, 24, 33-34, 36-41, 53-57, 76-80, 83, 85-89, 93-96, 109, 120-234, 258, 260, 263-265, 267-272, 275, 278-282, 317-335, 337-365, 367-384, 386-387, 389-410, 415-422, 426, 428-439, 443, 447, 449-463, 467-472, 475, 580-581, 584-585, 588-591, 597-622, 635-654, 656-683, 686, 724, 728-757 | -| `src/renderer/src/store/slices/browser.test.ts` | 12, 184-199 | -| `src/renderer/src/store/slices/browser.ts` | 16, 422-423, 425 | -| `src/renderer/src/store/slices/editor.test.ts` | 12, 255-282 | -| `src/renderer/src/store/slices/editor.ts` | 28, 498-519, 849, 1059 | - -## Review Standards Reference - -- Follow `/review-code` standards -- Follow `/review-algorithm-architecture` standards -- Focus on correctness, security, performance, maintainability -- Priority levels: Critical > High > Medium > Low - -## File Categories - -### Frontend/UI - -- `src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx` -- `src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx` -- `src/renderer/src/store/slices/browser.test.ts` -- `src/renderer/src/store/slices/browser.ts` -- `src/renderer/src/store/slices/editor.test.ts` -- `src/renderer/src/store/slices/editor.ts` - -## Skipped Issues (Do Not Re-validate) - - - - - -None. - -## Iteration State - -Current iteration: 1 -Last completed phase: Setup -Files fixed this iteration: [] diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx index f2a7390984a..e6c3e3e7bf0 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx @@ -26,12 +26,18 @@ type FloatingPanelStoreState = { worktreeId: string, groupId?: string, shellOverride?: string, - options?: { activate?: boolean } + options?: { activate?: boolean; pendingActivationSpawn?: boolean; initialPtyId?: string } ) => TerminalTab createBrowserTab: ( worktreeId: string, url: string, - options?: { title?: string; targetGroupId?: string } + options?: { + activate?: boolean + focusAddressBar?: boolean + sessionProfileId?: string | null + title?: string + targetGroupId?: string + } ) => BrowserTab closeTab: (tabId: string) => void closeBrowserTab: (tabId: string) => void @@ -46,7 +52,7 @@ type FloatingPanelStoreState = { openFile: (file: unknown, options?: unknown) => void browserDefaultUrl: string tabBarOrderByWorktree: Record - settings: { floatingTerminalCwd?: string } + settings: { activeRuntimeEnvironmentId?: string | null; floatingTerminalCwd?: string } } const hookRuntime = vi.hoisted(() => ({ @@ -61,14 +67,19 @@ const storeBox = vi.hoisted(() => ({ const mocks = vi.hoisted(() => ({ activateTab: vi.fn(), + activateWebRuntimeSessionTab: vi.fn(), closeBrowserTab: vi.fn(), + closeWebRuntimeSessionTab: vi.fn(), closeFile: vi.fn(), closeTab: vi.fn(), createBrowserTab: vi.fn(), createTab: vi.fn(), + createWebRuntimeSessionBrowserTab: vi.fn(), + createWebRuntimeSessionTerminal: vi.fn(), focusTerminalTabSurface: vi.fn(), getFloatingTerminalCwd: vi.fn(), getInstallStatus: vi.fn(), + isWebRuntimeSessionActive: vi.fn(), markFileDirty: vi.fn(), openFile: vi.fn(), pinFile: vi.fn(), @@ -78,6 +89,10 @@ const mocks = vi.hoisted(() => ({ setTabPaneExpanded: vi.fn() })) +const saveDialogBox = vi.hoisted(() => ({ + fileId: null as string | null +})) + vi.mock('react', async () => { const actual = await vi.importActual('react') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() return { @@ -177,15 +192,46 @@ vi.mock('@/components/ui/dialog', () => ({ vi.mock('@/components/terminal/useTerminalSaveDialog', () => ({ useTerminalSaveDialog: () => ({ - handleSaveDialogCancel: vi.fn(), - handleSaveDialogDiscard: vi.fn(), - handleSaveDialogSave: vi.fn(), - requestCloseFile: mocks.closeFile, - saveDialogFile: null, - saveDialogFileId: null + handleSaveDialogCancel: () => { + saveDialogBox.fileId = null + }, + handleSaveDialogDiscard: () => { + if (saveDialogBox.fileId) { + mocks.markFileDirty(saveDialogBox.fileId, false) + mocks.closeFile(saveDialogBox.fileId) + } + saveDialogBox.fileId = null + }, + handleSaveDialogSave: () => { + saveDialogBox.fileId = null + }, + requestCloseFile: (fileId: string) => { + const file = (storeBox.state as FloatingPanelStoreState).openFiles.find( + (candidate) => candidate.id === fileId + ) + if (file?.isDirty) { + saveDialogBox.fileId = fileId + return + } + mocks.closeFile(fileId) + }, + saveDialogFile: saveDialogBox.fileId + ? ((storeBox.state as FloatingPanelStoreState).openFiles.find( + (file) => file.id === saveDialogBox.fileId + ) ?? null) + : null, + saveDialogFileId: saveDialogBox.fileId }) })) +vi.mock('@/runtime/web-runtime-session', () => ({ + activateWebRuntimeSessionTab: mocks.activateWebRuntimeSessionTab, + closeWebRuntimeSessionTab: mocks.closeWebRuntimeSessionTab, + createWebRuntimeSessionBrowserTab: mocks.createWebRuntimeSessionBrowserTab, + createWebRuntimeSessionTerminal: mocks.createWebRuntimeSessionTerminal, + isWebRuntimeSessionActive: mocks.isWebRuntimeSessionActive +})) + vi.mock('@/lib/connection-context', () => ({ getConnectionId: () => undefined })) @@ -258,6 +304,20 @@ function makeTab(overrides: Partial = {}): TerminalTab { } } +function makeFile(overrides: Partial = {}): OpenFile { + const id = overrides.id ?? 'file-1' + return { + id, + filePath: overrides.filePath ?? `/tmp/orca/${id}.md`, + relativePath: overrides.relativePath ?? `${id}.md`, + worktreeId: overrides.worktreeId ?? FLOATING_TERMINAL_WORKTREE_ID, + language: overrides.language ?? 'markdown', + isDirty: overrides.isDirty ?? false, + mode: overrides.mode ?? 'edit', + ...overrides + } +} + function setFloatingTabs(tabs: TerminalTab[]): void { const state = storeBox.state as FloatingPanelStoreState const groupId = 'floating-group' @@ -290,6 +350,36 @@ function setFloatingTabs(tabs: TerminalTab[]): void { state.tabBarOrderByWorktree = { [FLOATING_TERMINAL_WORKTREE_ID]: tabs.map((tab) => tab.id) } } +function setFloatingEditorTabs(files: OpenFile[]): void { + const state = storeBox.state as FloatingPanelStoreState + const groupId = 'floating-group' + const unifiedTabs = files.map((file, index) => ({ + id: `tab-${file.id}`, + entityId: file.id, + groupId, + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + contentType: 'editor', + label: file.relativePath, + customLabel: null, + color: null, + sortOrder: index, + createdAt: index + })) + state.openFiles = files + state.unifiedTabsByWorktree = { [FLOATING_TERMINAL_WORKTREE_ID]: unifiedTabs } + state.groupsByWorktree = { + [FLOATING_TERMINAL_WORKTREE_ID]: [ + { + id: groupId, + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + activeTabId: unifiedTabs[0]?.id ?? null, + tabOrder: unifiedTabs.map((tab) => tab.id), + recentTabIds: unifiedTabs.map((tab) => tab.id) + } + ] + } +} + function resetStore(tabs: TerminalTab[] = []): void { storeBox.state = { tabsByWorktree: { [FLOATING_TERMINAL_WORKTREE_ID]: tabs }, @@ -361,6 +451,11 @@ function runEffects(): void { } } +async function flushAsyncWork(): Promise { + await Promise.resolve() + await Promise.resolve() +} + async function renderPanel(open: boolean, onOpenChange = vi.fn()): Promise { hookRuntime.index = 0 const { FloatingTerminalPanel } = await import('./FloatingTerminalPanel') @@ -373,14 +468,19 @@ describe('FloatingTerminalPanel close behavior', () => { hookRuntime.effects = [] hookRuntime.index = 0 hookRuntime.values = [] + saveDialogBox.fileId = null resetStore() mocks.createTab.mockReturnValue(makeTab({ id: 'created-tab' })) + mocks.createWebRuntimeSessionBrowserTab.mockResolvedValue(false) + mocks.createWebRuntimeSessionTerminal.mockResolvedValue(false) mocks.getFloatingTerminalCwd.mockResolvedValue('/tmp/orca') mocks.getInstallStatus.mockResolvedValue({ state: 'installed' }) + mocks.isWebRuntimeSessionActive.mockReturnValue(false) vi.stubGlobal('window', { addEventListener: vi.fn(), api: { app: { getFloatingTerminalCwd: mocks.getFloatingTerminalCwd }, + browser: { notifyActiveTabChanged: vi.fn() }, cli: { getInstallStatus: mocks.getInstallStatus } }, innerWidth: 1200, @@ -396,25 +496,53 @@ describe('FloatingTerminalPanel close behavior', () => { it('bootstraps a terminal tab only when the panel opens', async () => { await renderPanel(false) runEffects() + await flushAsyncWork() expect(mocks.createTab).not.toHaveBeenCalled() await renderPanel(true) runEffects() + await flushAsyncWork() expect(mocks.createTab).toHaveBeenCalledTimes(1) - expect(mocks.createTab).toHaveBeenCalledWith(FLOATING_TERMINAL_WORKTREE_ID) + expect(mocks.createTab).toHaveBeenCalledWith( + FLOATING_TERMINAL_WORKTREE_ID, + undefined, + undefined, + { activate: false } + ) + expect(mocks.activateTab).toHaveBeenCalledWith('created-tab') expect(mocks.focusTerminalTabSurface).toHaveBeenCalledWith('created-tab') await renderPanel(true) runEffects() + await flushAsyncWork() expect(mocks.createTab).toHaveBeenCalledTimes(1) await renderPanel(false) runEffects() await renderPanel(true) runEffects() + await flushAsyncWork() expect(mocks.createTab).toHaveBeenCalledTimes(2) }) + it('creates new floating terminal tabs without globally activating createTab', async () => { + setFloatingTabs([makeTab({ id: 'tab-1' })]) + + const element = await renderPanel(true) + const tabBar = findByTypeName(element, 'TabBar') + ;(tabBar.props.onNewTerminalTab as () => void)() + await flushAsyncWork() + + expect(mocks.createTab).toHaveBeenCalledWith( + FLOATING_TERMINAL_WORKTREE_ID, + 'floating-group', + undefined, + { activate: false } + ) + expect(mocks.activateTab).toHaveBeenCalledWith('created-tab') + expect(mocks.focusTerminalTabSurface).toHaveBeenCalledWith('created-tab') + }) + it('closes the panel when the explicit close action removes the last tab', async () => { const onOpenChange = vi.fn() setFloatingTabs([makeTab({ id: 'tab-1' })]) @@ -462,6 +590,74 @@ describe('FloatingTerminalPanel close behavior', () => { expect(onOpenChange).toHaveBeenCalledWith(false) }) + it('routes floating terminal create and close through active web runtime sessions', async () => { + const onOpenChange = vi.fn() + setFloatingTabs([makeTab({ id: 'tab-1' })]) + ;(storeBox.state as FloatingPanelStoreState).settings.activeRuntimeEnvironmentId = 'runtime-1' + mocks.isWebRuntimeSessionActive.mockReturnValue(true) + mocks.createWebRuntimeSessionTerminal.mockResolvedValue(true) + + const element = await renderPanel(true, onOpenChange) + const tabBar = findByTypeName(element, 'TabBar') + ;(tabBar.props.onNewTerminalTab as () => void)() + await flushAsyncWork() + + expect(mocks.createWebRuntimeSessionTerminal).toHaveBeenCalledWith({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + targetGroupId: 'floating-group', + command: undefined, + activate: true, + selectWorktree: false + }) + expect(mocks.createTab).not.toHaveBeenCalled() + + ;(tabBar.props.onClose as (tabId: string) => void)('tab-1') + expect(mocks.closeWebRuntimeSessionTab).toHaveBeenCalledWith({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + tabId: 'tab-1', + environmentId: 'runtime-1' + }) + expect(mocks.closeTab).not.toHaveBeenCalled() + expect(onOpenChange).not.toHaveBeenCalled() + }) + + it('queues dirty editor closes from close-all-files instead of overwriting the dialog id', async () => { + setFloatingEditorTabs([ + makeFile({ id: 'file-a', isDirty: true }), + makeFile({ id: 'file-b', isDirty: true }) + ]) + + const element = await renderPanel(true) + const tabBar = findByTypeName(element, 'TabBar') + ;(tabBar.props.onCloseAllFiles as () => void)() + + expect(saveDialogBox.fileId).toBe('file-a') + expect(mocks.closeFile).not.toHaveBeenCalledWith('file-a') + expect(mocks.closeFile).not.toHaveBeenCalledWith('file-b') + }) + + it('queues dirty editor closes from close-others and close-to-right one file at a time', async () => { + setFloatingEditorTabs([ + makeFile({ id: 'file-a', isDirty: true }), + makeFile({ id: 'file-b', isDirty: true }), + makeFile({ id: 'file-c', isDirty: true }) + ]) + + const element = await renderPanel(true) + const tabBar = findByTypeName(element, 'TabBar') + ;(tabBar.props.onCloseOthers as (tabId: string) => void)('tab-file-b') + expect(saveDialogBox.fileId).toBe('file-a') + + saveDialogBox.fileId = null + mocks.closeFile.mockClear() + hookRuntime.values = [] + const nextElement = await renderPanel(true) + const nextTabBar = findByTypeName(nextElement, 'TabBar') + ;(nextTabBar.props.onCloseToRight as (tabId: string) => void)('tab-file-a') + expect(saveDialogBox.fileId).toBe('file-b') + expect(mocks.closeFile).not.toHaveBeenCalledWith('file-c') + }) + it('reads the current tab list for bulk close actions', async () => { setFloatingTabs([makeTab({ id: 'old-left' }), makeTab({ id: 'old-keep' })]) diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index e5518bccbc4..ae367fa27ff 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -18,6 +18,7 @@ import { DialogTitle } from '@/components/ui/dialog' import { useTerminalSaveDialog } from '@/components/terminal/useTerminalSaveDialog' +import { appendUniqueOpenFileIds } from '@/components/terminal/unsaved-close-queue' import { getConnectionId } from '@/lib/connection-context' import { createUntitledMarkdownFile } from '@/lib/create-untitled-markdown' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' @@ -32,6 +33,13 @@ import { import { useAppStore } from '@/store' import type { OpenFile } from '@/store/slices/editor' import { destroyWorkspaceWebviews } from '@/store/slices/browser-webview-cleanup' +import { + activateWebRuntimeSessionTab, + closeWebRuntimeSessionTab, + createWebRuntimeSessionBrowserTab, + createWebRuntimeSessionTerminal, + isWebRuntimeSessionActive +} from '@/runtime/web-runtime-session' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import type { BrowserTab as BrowserTabState, @@ -74,7 +82,6 @@ export function FloatingTerminalPanel({ }: FloatingTerminalPanelProps): React.JSX.Element | null { const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) - const browserPagesByWorkspace = useAppStore((s) => s.browserPagesByWorkspace) const groupsByWorktree = useAppStore((s) => s.groupsByWorktree) const unifiedTabsByWorktree = useAppStore((s) => s.unifiedTabsByWorktree) const openFiles = useAppStore((s) => s.openFiles) @@ -107,6 +114,8 @@ export function FloatingTerminalPanel({ const normalizedInitialBoundsRef = useRef(false) const previousOpenRef = useRef(false) const pendingLastEditorCloseRef = useRef(false) + const pendingEditorCloseQueueRef = useRef([]) + const saveDialogFileIdRef = useRef(null) const panelRef = useRef(null) const dragRef = useRef<{ pointerId: number @@ -228,6 +237,84 @@ export function FloatingTerminalPanel({ handleSaveDialogCancel } = useTerminalSaveDialog({ openFiles, closeFile, markFileDirty }) + const getNextQueuedEditorClose = useCallback((): string | null => { + while (pendingEditorCloseQueueRef.current.length > 0) { + const fileId = pendingEditorCloseQueueRef.current[0] + const file = useAppStore.getState().openFiles.find((candidate) => candidate.id === fileId) + if (!file) { + pendingEditorCloseQueueRef.current.shift() + continue + } + if (!file.isDirty) { + closeFile(fileId) + pendingEditorCloseQueueRef.current.shift() + continue + } + return fileId + } + return null + }, [closeFile]) + + const advanceEditorCloseQueue = useCallback(() => { + if (saveDialogFileIdRef.current !== null) { + return + } + const nextFileId = getNextQueuedEditorClose() + if (!nextFileId) { + return + } + // Why: useTerminalSaveDialog only stores one file id. Mark the next id as + // reserved before setting dialog state so same-tick bulk close requests + // cannot overwrite it with a later dirty tab. + saveDialogFileIdRef.current = nextFileId + requestCloseFile(nextFileId) + }, [getNextQueuedEditorClose, requestCloseFile]) + + const queueEditorCloseRequests = useCallback( + (fileIds: string[]) => { + pendingEditorCloseQueueRef.current = appendUniqueOpenFileIds( + pendingEditorCloseQueueRef.current, + fileIds, + new Set(useAppStore.getState().openFiles.map((file) => file.id)) + ) + advanceEditorCloseQueue() + }, + [advanceEditorCloseQueue] + ) + + useEffect(() => { + saveDialogFileIdRef.current = saveDialogFileId + if (saveDialogFileId === null) { + advanceEditorCloseQueue() + } + }, [advanceEditorCloseQueue, saveDialogFileId]) + + const handleFloatingSaveDialogSave = useCallback(() => { + const fileId = saveDialogFileIdRef.current + if (fileId) { + pendingEditorCloseQueueRef.current = pendingEditorCloseQueueRef.current.filter( + (queuedId) => queuedId !== fileId + ) + } + handleSaveDialogSave() + }, [handleSaveDialogSave]) + + const handleFloatingSaveDialogDiscard = useCallback(() => { + const fileId = saveDialogFileIdRef.current + if (fileId) { + pendingEditorCloseQueueRef.current = pendingEditorCloseQueueRef.current.filter( + (queuedId) => queuedId !== fileId + ) + } + void Promise.resolve(handleSaveDialogDiscard()) + }, [handleSaveDialogDiscard]) + + const handleFloatingSaveDialogCancel = useCallback(() => { + pendingEditorCloseQueueRef.current = [] + pendingLastEditorCloseRef.current = false + handleSaveDialogCancel() + }, [handleSaveDialogCancel]) + useEffect(() => { if (!open || normalizedInitialBoundsRef.current || typeof window === 'undefined') { return @@ -255,9 +342,24 @@ export function FloatingTerminalPanel({ if (!opened || unifiedTabs.length > 0) { return } - const tab = createTab(FLOATING_TERMINAL_WORKTREE_ID) - focusTerminalTabSurface(tab.id) - }, [createTab, open, unifiedTabs.length]) + void (async () => { + if ( + await createWebRuntimeSessionTerminal({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + targetGroupId: activeGroup?.id, + activate: true, + selectWorktree: false + }) + ) { + return + } + const tab = createTab(FLOATING_TERMINAL_WORKTREE_ID, activeGroup?.id, undefined, { + activate: false + }) + activateTab(tab.id) + focusTerminalTabSurface(tab.id) + })() + }, [activateTab, activeGroup, createTab, open, unifiedTabs.length]) useEffect(() => { if (!open || !activeTerminalId) { @@ -316,10 +418,27 @@ export function FloatingTerminalPanel({ return } activateTab(item.id) + const runtimeEnvironmentId = useAppStore + .getState() + .settings?.activeRuntimeEnvironmentId?.trim() if (item.contentType === 'terminal') { + if (isWebRuntimeSessionActive(runtimeEnvironmentId)) { + void activateWebRuntimeSessionTab({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + tabId: item.entityId, + environmentId: runtimeEnvironmentId + }) + } setActiveTab(item.entityId) focusTerminalTabSurface(item.entityId) } else if (item.contentType === 'browser') { + if (isWebRuntimeSessionActive(runtimeEnvironmentId)) { + void activateWebRuntimeSessionTab({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + tabId: item.id, + environmentId: runtimeEnvironmentId + }) + } const workspace = useAppStore .getState() .browserTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID]?.find( @@ -335,21 +454,47 @@ export function FloatingTerminalPanel({ const createFloatingTerminalTab = useCallback( (shellOverride?: string) => { - const tab = createTab(FLOATING_TERMINAL_WORKTREE_ID, activeGroup?.id, shellOverride, { - activate: true - }) - focusTerminalTabSurface(tab.id) + void (async () => { + if ( + await createWebRuntimeSessionTerminal({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + targetGroupId: activeGroup?.id, + command: shellOverride, + activate: true, + selectWorktree: false + }) + ) { + return + } + const tab = createTab(FLOATING_TERMINAL_WORKTREE_ID, activeGroup?.id, shellOverride, { + activate: false + }) + activateTab(tab.id) + focusTerminalTabSurface(tab.id) + })() }, - [activeGroup, createTab] + [activateTab, activeGroup, createTab] ) const createFloatingBrowserTab = useCallback(() => { - const url = browserDefaultUrl ?? 'about:blank' - createBrowserTab(FLOATING_TERMINAL_WORKTREE_ID, url, { - title: 'New Browser Tab', - focusAddressBar: true, - targetGroupId: activeGroup?.id - }) + void (async () => { + const url = browserDefaultUrl ?? 'about:blank' + if ( + await createWebRuntimeSessionBrowserTab({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + url, + targetGroupId: activeGroup?.id, + selectWorktree: false + }) + ) { + return + } + createBrowserTab(FLOATING_TERMINAL_WORKTREE_ID, url, { + title: 'New Browser Tab', + focusAddressBar: true, + targetGroupId: activeGroup?.id + }) + })() }, [activeGroup, browserDefaultUrl, createBrowserTab]) const createFloatingMarkdownTab = useCallback(() => { @@ -375,43 +520,71 @@ export function FloatingTerminalPanel({ })() }, [activeGroup, cwd, openFile, settings]) - const closeFloatingItem = useCallback( - (visibleId: string) => { + const closeFloatingItems = useCallback( + (visibleIds: string[]) => { const state = useAppStore.getState() const currentGroupTabs = activeGroup ? (state.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []).filter( (tab) => tab.groupId === activeGroup.id ) : (state.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []) - const item = resolveGroupTabFromVisibleId(currentGroupTabs, visibleId) - if (!item) { + const items = visibleIds + .map((visibleId) => resolveGroupTabFromVisibleId(currentGroupTabs, visibleId)) + .filter((item): item is Tab => item !== null) + if (items.length === 0) { return } - const isClosingLastTab = currentGroupTabs.length === 1 - if (item.contentType === 'terminal') { - closeTab(item.entityId) - } else if (item.contentType === 'browser') { - destroyWorkspaceWebviews(browserPagesByWorkspace, item.entityId) - closeBrowserTab(item.entityId) - } else { - requestCloseFile(item.entityId) - if (state.openFiles.find((file) => file.id === item.entityId)?.isDirty) { - pendingLastEditorCloseRef.current = isClosingLastTab - return + const closingTabIds = new Set(items.map((item) => item.id)) + const isClosingEveryVisibleTab = currentGroupTabs.every((tab) => closingTabIds.has(tab.id)) + const dirtyEditorFileIds: string[] = [] + let hasRuntimeSessionClose = false + const runtimeEnvironmentId = state.settings?.activeRuntimeEnvironmentId?.trim() + for (const item of items) { + if ( + (item.contentType === 'terminal' || item.contentType === 'browser') && + isWebRuntimeSessionActive(runtimeEnvironmentId) + ) { + // Why: paired web clients mirror host-owned tabs; ask the runtime to + // close the host tab instead of deleting the local mirror directly. + hasRuntimeSessionClose = true + void closeWebRuntimeSessionTab({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + tabId: item.contentType === 'browser' ? item.id : item.entityId, + environmentId: runtimeEnvironmentId + }) + continue + } + if (item.contentType === 'terminal') { + closeTab(item.entityId) + } else if (item.contentType === 'browser') { + destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId) + closeBrowserTab(item.entityId) + } else { + const file = state.openFiles.find((candidate) => candidate.id === item.entityId) + if (file?.isDirty) { + dirtyEditorFileIds.push(item.entityId) + continue + } + closeFile(item.entityId) } } - if (isClosingLastTab) { + if (dirtyEditorFileIds.length > 0) { + pendingLastEditorCloseRef.current = isClosingEveryVisibleTab + queueEditorCloseRequests(dirtyEditorFileIds) + return + } + if (isClosingEveryVisibleTab && !hasRuntimeSessionClose) { onOpenChange(false) } }, - [ - activeGroup, - browserPagesByWorkspace, - closeBrowserTab, - closeTab, - onOpenChange, - requestCloseFile - ] + [activeGroup, closeBrowserTab, closeFile, closeTab, onOpenChange, queueEditorCloseRequests] + ) + + const closeFloatingItem = useCallback( + (visibleId: string) => { + closeFloatingItems([visibleId]) + }, + [closeFloatingItems] ) const closeOthers = useCallback( @@ -426,13 +599,11 @@ export function FloatingTerminalPanel({ if (!item) { return } - for (const tab of currentGroupTabs) { - if (tab.id !== item.id && !tab.isPinned) { - closeFloatingItem(tab.id) - } - } + closeFloatingItems( + currentGroupTabs.filter((tab) => tab.id !== item.id && !tab.isPinned).map((tab) => tab.id) + ) }, - [activeGroup, closeFloatingItem] + [activeGroup, closeFloatingItems] ) const closeToRight = useCallback( @@ -457,16 +628,32 @@ export function FloatingTerminalPanel({ return } const tabById = new Map(currentGroupTabs.map((tab) => [tab.id, tab])) - for (const tabId of currentGroup.tabOrder.slice(index + 1)) { - const tab = tabById.get(tabId) - if (tab && !tab.isPinned) { - closeFloatingItem(tab.id) - } - } + closeFloatingItems( + currentGroup.tabOrder.slice(index + 1).filter((tabId) => { + const tab = tabById.get(tabId) + return tab ? !tab.isPinned : false + }) + ) }, - [activeGroup, closeFloatingItem] + [activeGroup, closeFloatingItems] ) + const closeAllFiles = useCallback(() => { + const state = useAppStore.getState() + const currentGroupTabs = activeGroup + ? (state.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []).filter( + (tab) => tab.groupId === activeGroup.id + ) + : (state.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []) + closeFloatingItems( + currentGroupTabs + .filter( + (tab) => tab.contentType !== 'terminal' && tab.contentType !== 'browser' && !tab.isPinned + ) + .map((tab) => tab.id) + ) + }, [activeGroup, closeFloatingItems]) + const toggleMaximized = useCallback(() => { setMaximized((current) => { if (current) { @@ -596,21 +783,30 @@ export function FloatingTerminalPanel({ onActivateBrowserTab={activateFloatingItem} onCloseBrowserTab={closeFloatingItem} onDuplicateBrowserTab={(browserTabId) => { - const source = browserTabs.find((tab) => tab.id === browserTabId) - if (!source) { - return - } - createBrowserTab(FLOATING_TERMINAL_WORKTREE_ID, source.url, { - title: source.title, - sessionProfileId: source.sessionProfileId, - targetGroupId: activeGroup?.id - }) - }} - onCloseAllFiles={() => { - for (const file of editorItems) { - closeFloatingItem(file.tabId) - } + void (async () => { + const source = browserTabs.find((tab) => tab.id === browserTabId) + if (!source) { + return + } + if ( + await createWebRuntimeSessionBrowserTab({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + url: source.url, + profileId: source.sessionProfileId, + targetGroupId: activeGroup?.id, + selectWorktree: false + }) + ) { + return + } + createBrowserTab(FLOATING_TERMINAL_WORKTREE_ID, source.url, { + title: source.title, + sessionProfileId: source.sessionProfileId, + targetGroupId: activeGroup?.id + }) + })() }} + onCloseAllFiles={closeAllFiles} onPinFile={pinFile} tabBarOrder={tabBarOrder} /> @@ -721,7 +917,7 @@ export function FloatingTerminalPanel({ open={saveDialogFileId !== null} onOpenChange={(nextOpen) => { if (!nextOpen) { - handleSaveDialogCancel() + handleFloatingSaveDialogCancel() } }} > @@ -735,13 +931,23 @@ export function FloatingTerminalPanel({ - - - diff --git a/src/renderer/src/runtime/web-runtime-session.test.ts b/src/renderer/src/runtime/web-runtime-session.test.ts index 4cd3433ceac..eb18d5fbe45 100644 --- a/src/renderer/src/runtime/web-runtime-session.test.ts +++ b/src/renderer/src/runtime/web-runtime-session.test.ts @@ -201,6 +201,48 @@ describe('createWebRuntimeSessionBrowserTab', () => { expect(setStateResults.at(-1)).toEqual({ state: 'after' }) }) + it('can create a browser tab without selecting the target worktree', async () => { + const setStateResults: unknown[] = [] + mocks.setState.mockImplementation((updater: (state: unknown) => unknown) => { + const result = updater({ + state: 'before-stage', + activeWorktreeId: 'main-worktree' + }) + setStateResults.push(result) + }) + const runtimeCall = vi + .fn() + .mockResolvedValueOnce({ + id: 'create', + ok: true, + result: { browserPageId: 'remote-browser-page-1' } + }) + .mockResolvedValueOnce({ + id: 'list', + ok: true, + result: makeSnapshot() + }) + + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall + } + } + }) + + await expect( + createWebRuntimeSessionBrowserTab({ + worktreeId: WORKTREE_ID, + url: 'https://example.com/', + selectWorktree: false + }) + ).resolves.toBe(true) + + expect(setStateResults).not.toContainEqual({ activeWorktreeId: WORKTREE_ID }) + expect(mocks.focusBrowserTabInWorktree).not.toHaveBeenCalled() + }) + it('does not focus a staged browser tab when the user leaves before host create resolves', async () => { let activeWorktreeId = WORKTREE_ID mocks.getState.mockImplementation(() => ({ @@ -395,6 +437,60 @@ describe('createWebRuntimeSessionTerminal', () => { ENVIRONMENT_ID ) }) + + it('can create a terminal without selecting the target worktree', async () => { + const setStateResults: unknown[] = [] + mocks.setState.mockImplementation((updater: (state: unknown) => unknown) => { + const result = updater({ + state: 'before', + activeWorktreeId: 'main-worktree' + }) + setStateResults.push(result) + }) + const runtimeCall = vi + .fn() + .mockResolvedValueOnce({ + id: 'create-terminal', + ok: true, + result: { + tab: { + type: 'terminal', + id: 'host-tab-2::leaf-1', + parentTabId: 'host-tab-2', + leafId: 'leaf-1', + title: 'Terminal 2', + terminal: 'pty-2', + status: 'ready', + isActive: true + }, + publicationEpoch: 'epoch-1', + snapshotVersion: 2 + } + }) + .mockResolvedValueOnce({ + id: 'list', + ok: true, + result: makeSnapshot() + }) + + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall + } + } + }) + + await expect( + createWebRuntimeSessionTerminal({ + worktreeId: WORKTREE_ID, + activate: true, + selectWorktree: false + }) + ).resolves.toBe(true) + + expect(setStateResults).not.toContainEqual({ activeWorktreeId: WORKTREE_ID }) + }) }) describe('moveWebRuntimeSessionTab', () => { diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index 224f1d0300a..8a4bb0f8137 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -39,6 +39,7 @@ export async function createWebRuntimeSessionTerminal(args: { targetGroupId?: string command?: string activate?: boolean + selectWorktree?: boolean }): Promise { const environmentId = args.environmentId?.trim() ?? @@ -48,7 +49,9 @@ export async function createWebRuntimeSessionTerminal(args: { return false } - selectWebRuntimeSessionWorktree(args.worktreeId) + if (args.selectWorktree !== false) { + selectWebRuntimeSessionWorktree(args.worktreeId) + } try { const response = await window.api.runtimeEnvironments.call({ selector: environmentId, @@ -80,6 +83,7 @@ export async function createWebRuntimeSessionBrowserTab(args: { url?: string profileId?: string | null targetGroupId?: string + selectWorktree?: boolean }): Promise { const environmentId = args.environmentId?.trim() ?? @@ -89,8 +93,11 @@ export async function createWebRuntimeSessionBrowserTab(args: { return false } + const shouldSelectWorktree = args.selectWorktree !== false const stagedFromWorktreeId = useAppStore.getState().activeWorktreeId - selectWebRuntimeSessionWorktree(args.worktreeId) + if (shouldSelectWorktree) { + selectWebRuntimeSessionWorktree(args.worktreeId) + } try { const response = await window.api.runtimeEnvironments.call({ selector: environmentId, @@ -114,8 +121,9 @@ export async function createWebRuntimeSessionBrowserTab(args: { url: args.url, targetGroupId: args.targetGroupId, restoreFocus: - stagedFromWorktreeId === args.worktreeId || - useAppStore.getState().activeWorktreeId === args.worktreeId + shouldSelectWorktree && + (stagedFromWorktreeId === args.worktreeId || + useAppStore.getState().activeWorktreeId === args.worktreeId) }) void refreshWebRuntimeSessionTabsSnapshot(environmentId, args.worktreeId) return true diff --git a/src/renderer/src/store/slices/browser.test.ts b/src/renderer/src/store/slices/browser.test.ts index 15f0b82e9cc..42e3b60f1d9 100644 --- a/src/renderer/src/store/slices/browser.test.ts +++ b/src/renderer/src/store/slices/browser.test.ts @@ -182,16 +182,23 @@ describe('createBrowserSlice annotations', () => { }) describe('createBrowserSlice floating tabs', () => { - it('focuses the address bar for new floating browser tabs without changing the main surface', () => { + it('tracks new floating browser tabs without changing the main browser surface', () => { const store = createTestStore() store.setState({ activeWorktreeId: 'wt-1', activeTabType: 'terminal' } as Partial) + const mainTab = store.getState().createBrowserTab('wt-1', 'https://example.com') + const activeTabTypeBeforeFloating = store.getState().activeTabType const tab = store.getState().createBrowserTab(FLOATING_TERMINAL_WORKTREE_ID, 'about:blank', { focusAddressBar: true }) + expect(store.getState().activeBrowserTabId).toBe(mainTab.id) + expect(store.getState().activeBrowserTabIdByWorktree['wt-1']).toBe(mainTab.id) + expect(store.getState().activeBrowserTabIdByWorktree[FLOATING_TERMINAL_WORKTREE_ID]).toBe( + tab.id + ) expect(store.getState().pendingAddressBarFocusByTabId[tab.id]).toBe(true) - expect(store.getState().activeTabType).toBe('terminal') + expect(store.getState().activeTabType).toBe(activeTabTypeBeforeFloating) }) }) diff --git a/src/renderer/src/store/slices/browser.ts b/src/renderer/src/store/slices/browser.ts index 2244d6f0459..ef82c2fda71 100644 --- a/src/renderer/src/store/slices/browser.ts +++ b/src/renderer/src/store/slices/browser.ts @@ -438,7 +438,7 @@ export const createBrowserSlice: StateCreator = ...s.tabBarOrderByWorktree, [worktreeId]: nextTabBarOrder }, - activeBrowserTabId: shouldActivate ? workspaceId : s.activeBrowserTabId, + activeBrowserTabId: shouldUpdateGlobalActiveSurface ? workspaceId : s.activeBrowserTabId, activeBrowserTabIdByWorktree: { ...s.activeBrowserTabIdByWorktree, [worktreeId]: shouldActivate