Fix floating mixed tab review issues

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-05-19 15:55:14 -04:00
co-authored by Orca
parent 98ab911e6e
commit e789a6fec7
7 changed files with 600 additions and 147 deletions
-60
View File
@@ -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)
<!-- In scope: issues on these lines OR caused by these changes. Out of scope: unrelated pre-existing issues -->
| 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)
<!-- Issues validated but deemed not worth fixing. Do not re-validate these in future iterations. -->
<!-- Format: [file:line-range] | [severity] | [reason skipped] | [issue summary] -->
<!-- NOTE: Skips should be RARE - only purely cosmetic issues with no functional impact -->
None.
## Iteration State
Current iteration: 1
Last completed phase: Setup
Files fixed this iteration: []
@@ -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<string, string[]>
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<typeof import('react')>('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> = {}): TerminalTab {
}
}
function makeFile(overrides: Partial<OpenFile> = {}): 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<Tab>((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<void> {
await Promise.resolve()
await Promise.resolve()
}
async function renderPanel(open: boolean, onOpenChange = vi.fn()): Promise<unknown> {
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' })])
@@ -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<string[]>([])
const saveDialogFileIdRef = useRef<string | null>(null)
const panelRef = useRef<HTMLDivElement>(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({
</DialogDescription>
</DialogHeader>
<DialogFooter className="gap-2">
<Button type="button" variant="outline" size="sm" onClick={handleSaveDialogCancel}>
<Button
type="button"
variant="outline"
size="sm"
onClick={handleFloatingSaveDialogCancel}
>
Cancel
</Button>
<Button type="button" variant="outline" size="sm" onClick={handleSaveDialogDiscard}>
<Button
type="button"
variant="outline"
size="sm"
onClick={handleFloatingSaveDialogDiscard}
>
Don&apos;t Save
</Button>
<Button type="button" size="sm" onClick={handleSaveDialogSave}>
<Button type="button" size="sm" onClick={handleFloatingSaveDialogSave}>
Save
</Button>
</DialogFooter>
@@ -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', () => {
@@ -39,6 +39,7 @@ export async function createWebRuntimeSessionTerminal(args: {
targetGroupId?: string
command?: string
activate?: boolean
selectWorktree?: boolean
}): Promise<boolean> {
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<boolean> {
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
@@ -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<AppState>)
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)
})
})
+1 -1
View File
@@ -438,7 +438,7 @@ export const createBrowserSlice: StateCreator<AppState, [], [], BrowserSlice> =
...s.tabBarOrderByWorktree,
[worktreeId]: nextTabBarOrder
},
activeBrowserTabId: shouldActivate ? workspaceId : s.activeBrowserTabId,
activeBrowserTabId: shouldUpdateGlobalActiveSurface ? workspaceId : s.activeBrowserTabId,
activeBrowserTabIdByWorktree: {
...s.activeBrowserTabIdByWorktree,
[worktreeId]: shouldActivate