diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index bea3d2fe419..a78445ec19f 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -54,9 +54,7 @@ function getManagedCommand(scriptPath: string): string { // collapse and the launcher fails with `command not found`. Emit forward // slashes — Windows accepts them in path arguments and bash leaves them // intact, so the same JSON value works through every shell layer. - return process.platform === 'win32' - ? scriptPath.replaceAll('\\', '/') - : `/bin/sh "${scriptPath}"` + return process.platform === 'win32' ? scriptPath.replaceAll('\\', '/') : `/bin/sh "${scriptPath}"` } function getManagedScript(): string { diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index c45cabf8416..2eeb2608b2f 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -10,6 +10,7 @@ import { findWorktreeById } from '../store/slices/worktree-helpers' import { createUntitledMarkdownFile } from '../lib/create-untitled-markdown' import { getConnectionId } from '../lib/connection-context' import { extractIpcErrorMessage } from '../lib/ipc-error' +import { basename } from '../lib/path' import { Dialog, DialogContent, @@ -22,8 +23,10 @@ import { Button } from '@/components/ui/button' import TabBar from './tab-bar/TabBar' import TerminalPane from './terminal-pane/TerminalPane' import { + ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, ORCA_EDITOR_SAVE_AND_CLOSE_EVENT, ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, + type EditorRequestFileCloseDetail, requestEditorSaveQuiesce } from './editor/editor-autosave' import { isUpdaterQuitAndInstallInProgress } from '@/lib/updater-beforeunload' @@ -39,10 +42,18 @@ import { getEffectiveLayoutForWorktree as getEffectiveLayout, anyMountedWorktreeHasLayout as computeAnyMountedWorktreeHasLayout } from './terminal/split-group-mount' +import { appendUniqueOpenFileIds } from './terminal/unsaved-close-queue' import CodexRestartChip from './CodexRestartChip' const EditorPanel = lazy(() => import('./editor/EditorPanel')) +// Why: after a close-dialog handler advances the queue and renders the next +// dialog, gate new handler runs for this long so a stray carry-over click +// from the prior dialog can't silently act on the new one. Short enough to +// feel responsive on a deliberate follow-up click; long enough to absorb the +// trailing edge of a physical double-click (~150 ms on most hardware). +const CLOSE_DIALOG_DEBOUNCE_MS = 200 + function Terminal(): React.JSX.Element | null { const allWorktrees = useAllWorktrees() const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) @@ -66,7 +77,6 @@ function Terminal(): React.JSX.Element | null { const setActiveFile = useAppStore((s) => s.setActiveFile) const openFile = useAppStore((s) => s.openFile) const closeFile = useAppStore((s) => s.closeFile) - const closeAllFiles = useAppStore((s) => s.closeAllFiles) const pinFile = useAppStore((s) => s.pinFile) const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) const createBrowserTab = useAppStore((s) => s.createBrowserTab) @@ -136,6 +146,24 @@ function Terminal(): React.JSX.Element | null { // Save confirmation dialog state const [saveDialogFileId, setSaveDialogFileId] = useState(null) const saveDialogFile = saveDialogFileId ? openFiles.find((f) => f.id === saveDialogFileId) : null + const pendingEditorCloseQueueRef = useRef([]) + + // Why: while a save-and-close is awaiting the file to disappear from + // openFiles, concurrent queueEditorCloseRequests calls (e.g. user clicks X + // on another dirty tab, or a split-group dispatch fires + // ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT) must not re-open the dialog over + // the in-flight save. Track the in-flight file here so + // getNextQueuedEditorClose can skip it as an un-advanceable head. + const inFlightSaveFileIdRef = useRef(null) + + // Why: after a Save/Discard/Cancel handler dismisses its dialog and advances + // the queue, a rapid second physical click can land on the freshly-rendered + // next dialog's button before the user has read the filename — silently + // discarding or saving work they didn't consciously choose to act on. Gate + // the three handlers on this ref and release after CLOSE_DIALOG_DEBOUNCE_MS + // so the stray click from the previous dialog is absorbed while a genuine + // new click on the next dialog still works. + const isClosingRef = useRef(false) // Window close confirmation dialog — shown when the user tries to close the // window (X button, Cmd+Q) while terminals with running processes exist. @@ -174,76 +202,136 @@ function Terminal(): React.JSX.Element | null { ) }, []) + const waitForFileClosed = useCallback((fileId: string, timeoutMs: number): Promise => { + if (!useAppStore.getState().openFiles.some((f) => f.id === fileId)) { + return Promise.resolve(true) + } + return new Promise((resolve) => { + let unsub: (() => void) | null = null + const timeoutId = window.setTimeout(() => { + unsub?.() + resolve(false) + }, timeoutMs) + unsub = useAppStore.subscribe((state) => { + if (!state.openFiles.some((f) => f.id === fileId)) { + window.clearTimeout(timeoutId) + unsub?.() + resolve(true) + } + }) + // Why: zustand only fires subscribers on subsequent state changes. If + // the file closed between the initial guard and subscribe, the + // transition was missed — re-check synchronously after subscribe. + if (!useAppStore.getState().openFiles.some((f) => f.id === fileId)) { + window.clearTimeout(timeoutId) + unsub?.() + resolve(true) + } + }) + }, []) + + const getNextQueuedEditorClose = useCallback((): string | null => { + // Why: bulk close actions can enqueue files that become clean or disappear + // before they reach the front. Drain those entries eagerly so the dialog + // only blocks on tabs that still require an explicit close decision. + while (pendingEditorCloseQueueRef.current.length > 0) { + const fileId = pendingEditorCloseQueueRef.current[0] + // Why: if a save is still in-flight for this fileId, do not re-open the + // dialog on top of it. waitForFileClosed will re-advance the queue once + // the file finishes closing (or the save times out). + if (inFlightSaveFileIdRef.current === fileId) { + return null + } + 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(() => { + const nextFileId = getNextQueuedEditorClose() + if (nextFileId) { + // Why: the queue can cross worktree boundaries during window-close + // flows. Switch to the target file's worktree before opening the + // dialog so the UI behind the dialog matches the filename in it. + const state = useAppStore.getState() + const file = state.openFiles.find((f) => f.id === nextFileId) + if (file && file.worktreeId !== state.activeWorktreeId) { + setActiveWorktree(file.worktreeId) + } + setActiveFile(nextFileId) + setActiveTabType('editor') + setSaveDialogFileId(nextFileId) + return + } + setSaveDialogFileId(null) + const pendingWindowClose = windowCloseAfterDirtyRef.current + if (pendingWindowClose) { + windowCloseAfterDirtyRef.current = null + proceedToNativeWindowClose(pendingWindowClose.isQuitting) + } + }, [ + getNextQueuedEditorClose, + proceedToNativeWindowClose, + setActiveFile, + setActiveTabType, + setActiveWorktree + ]) + + const queueEditorCloseRequests = useCallback( + (fileIds: string[], pendingWindowClose?: { isQuitting: boolean }) => { + if (pendingWindowClose) { + windowCloseAfterDirtyRef.current = pendingWindowClose + } + pendingEditorCloseQueueRef.current = appendUniqueOpenFileIds( + pendingEditorCloseQueueRef.current, + fileIds, + new Set(useAppStore.getState().openFiles.map((file) => file.id)) + ) + advanceEditorCloseQueue() + }, + [advanceEditorCloseQueue] + ) + const handleCloseFile = useCallback( (fileId: string) => { const file = useAppStore.getState().openFiles.find((f) => f.id === fileId) if (file?.isDirty) { - setSaveDialogFileId(fileId) + queueEditorCloseRequests([fileId]) return } closeFile(fileId) }, - [closeFile] + [closeFile, queueEditorCloseRequests] ) const handleSaveDialogSave = useCallback(async () => { + if (isClosingRef.current) { + return + } if (!saveDialogFileId) { return } + isClosingRef.current = true const fileId = saveDialogFileId - const pendingWindowClose = windowCloseAfterDirtyRef.current const file = useAppStore.getState().openFiles.find((f) => f.id === fileId) if (!file) { - setSaveDialogFileId(null) - windowCloseAfterDirtyRef.current = null - return - } - - if (pendingWindowClose) { - setSaveDialogFileId(null) - // Why: save-and-close must flush the latest draft even when the visible - // editor panel has already unmounted. The headless autosave controller - // owns that write path now, so the dialog signals it through a custom - // event instead of poking at editor component refs. - window.dispatchEvent( - new CustomEvent(ORCA_EDITOR_SAVE_AND_CLOSE_EVENT, { detail: { fileId } }) + pendingEditorCloseQueueRef.current = pendingEditorCloseQueueRef.current.filter( + (id) => id !== fileId ) - - const waitForFileClosed = (timeoutMs: number): Promise => { - if (!useAppStore.getState().openFiles.some((f) => f.id === fileId)) { - return Promise.resolve(true) - } - return new Promise((resolve) => { - let unsub: (() => void) | null = null - const timeoutId = window.setTimeout(() => { - unsub?.() - resolve(false) - }, timeoutMs) - unsub = useAppStore.subscribe((state) => { - if (!state.openFiles.some((f) => f.id === fileId)) { - window.clearTimeout(timeoutId) - unsub?.() - resolve(true) - } - }) - }) - } - - const closed = await waitForFileClosed(10_000) - if (!closed) { - toast.error('Save timed out or failed. Fix errors before closing.') - setSaveDialogFileId(fileId) - return - } - - const nextDirty = useAppStore.getState().openFiles.filter((f) => f.isDirty) - if (nextDirty.length > 0) { - setSaveDialogFileId(nextDirty[0].id) - } else { - const { isQuitting } = pendingWindowClose - windowCloseAfterDirtyRef.current = null - proceedToNativeWindowClose(isQuitting) - } + advanceEditorCloseQueue() + setTimeout(() => { + isClosingRef.current = false + }, CLOSE_DIALOG_DEBOUNCE_MS) return } @@ -251,55 +339,123 @@ function Terminal(): React.JSX.Element | null { // editor panel has already unmounted. The headless autosave controller // owns that write path now, so the dialog signals it through a custom // event instead of poking at editor component refs. - window.dispatchEvent(new CustomEvent(ORCA_EDITOR_SAVE_AND_CLOSE_EVENT, { detail: { fileId } })) setSaveDialogFileId(null) - }, [saveDialogFileId, proceedToNativeWindowClose]) + window.dispatchEvent(new CustomEvent(ORCA_EDITOR_SAVE_AND_CLOSE_EVENT, { detail: { fileId } })) + inFlightSaveFileIdRef.current = fileId + let closed = false + try { + closed = await waitForFileClosed(fileId, 10_000) + } finally { + // Why: clear the in-flight ref regardless of success/timeout so the + // queue head is no longer treated as un-advanceable by + // getNextQueuedEditorClose before we re-advance the queue below. + if (inFlightSaveFileIdRef.current === fileId) { + inFlightSaveFileIdRef.current = null + } + } + if (!closed) { + // Why: the save may have resolved in the tiny gap after the timeout + // fired. Re-check synchronously so we don't re-open a stale dialog + // for a file that is already gone — drain the queue entry and + // advance instead. Toast only for the genuine timeout case. + if (!useAppStore.getState().openFiles.some((f) => f.id === fileId)) { + pendingEditorCloseQueueRef.current = pendingEditorCloseQueueRef.current.filter( + (id) => id !== fileId + ) + advanceEditorCloseQueue() + setTimeout(() => { + isClosingRef.current = false + }, CLOSE_DIALOG_DEBOUNCE_MS) + return + } + toast.error('Save timed out or failed. Fix errors before closing.') + setSaveDialogFileId(fileId) + // Why: a genuine timeout leaves the user back on the same dialog, so + // release the guard immediately — a new click here is a deliberate + // retry, not a stray carry-over from a prior dialog. + isClosingRef.current = false + return + } + pendingEditorCloseQueueRef.current = pendingEditorCloseQueueRef.current.filter( + (id) => id !== fileId + ) + advanceEditorCloseQueue() + setTimeout(() => { + isClosingRef.current = false + }, CLOSE_DIALOG_DEBOUNCE_MS) + }, [advanceEditorCloseQueue, saveDialogFileId, waitForFileClosed]) const handleSaveDialogDiscard = useCallback(async () => { + if (isClosingRef.current) { + return + } if (!saveDialogFileId) { return } + isClosingRef.current = true const fileId = saveDialogFileId - const pendingWindowClose = windowCloseAfterDirtyRef.current - if (pendingWindowClose) { - // Why: autosave runs on a background timer. Wait for any pending/in-flight - // write to settle before honoring "Don't Save", otherwise the file can be - // written after the user explicitly chose to discard their edits. - try { - await requestEditorSaveQuiesce({ fileId }) - } catch { - // Quiesce failed — proceed with discard anyway so the user isn't stuck. - } - setSaveDialogFileId(null) - markFileDirty(fileId, false) - closeFile(fileId) - - const nextDirty = useAppStore.getState().openFiles.filter((f) => f.isDirty) - if (nextDirty.length > 0) { - setSaveDialogFileId(nextDirty[0].id) - } else { - const { isQuitting } = pendingWindowClose - windowCloseAfterDirtyRef.current = null - proceedToNativeWindowClose(isQuitting) - } - return - } + // Why: dismiss the dialog synchronously before awaiting quiesce. A rapid + // double-click on "Don't Save" would otherwise fire the handler twice + // with the same captured fileId, causing two concurrent queue advances + // after the quiesce settles. Mirrors handleSaveDialogSave's early clear. + setSaveDialogFileId(null) // Why: autosave runs on a background timer. Wait for any pending/in-flight // write to settle before honoring "Don't Save", otherwise the file can be // written after the user explicitly chose to discard their edits. - await requestEditorSaveQuiesce({ fileId }) + try { + await requestEditorSaveQuiesce({ fileId }) + } catch (error) { + // Why: quiesce failure must not trap the user in a close dialog loop, but + // silently swallowing it also hides broken autosave state. Warn so a + // stuck controller is visible in devtools instead of disappearing. + console.warn('Autosave quiesce failed before discard', error) + } markFileDirty(fileId, false) closeFile(fileId) - setSaveDialogFileId(null) - }, [saveDialogFileId, closeFile, markFileDirty, proceedToNativeWindowClose]) + pendingEditorCloseQueueRef.current = pendingEditorCloseQueueRef.current.filter( + (id) => id !== fileId + ) + advanceEditorCloseQueue() + setTimeout(() => { + isClosingRef.current = false + }, CLOSE_DIALOG_DEBOUNCE_MS) + }, [advanceEditorCloseQueue, closeFile, markFileDirty, saveDialogFileId]) const handleSaveDialogCancel = useCallback(() => { + if (isClosingRef.current) { + return + } + isClosingRef.current = true + pendingEditorCloseQueueRef.current = [] windowCloseAfterDirtyRef.current = null setSaveDialogFileId(null) + setTimeout(() => { + isClosingRef.current = false + }, CLOSE_DIALOG_DEBOUNCE_MS) }, []) + useEffect(() => { + const onRequestEditorClose = (event: Event): void => { + const customEvent = event as CustomEvent + const fileId = customEvent.detail?.fileId + if (!fileId) { + return + } + queueEditorCloseRequests([fileId]) + } + window.addEventListener( + ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, + onRequestEditorClose as EventListener + ) + return () => + window.removeEventListener( + ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, + onRequestEditorClose as EventListener + ) + }, [queueEditorCloseRequests]) + useEffect(() => { if (tabs.length === 0) { return @@ -591,6 +747,7 @@ function Terminal(): React.JSX.Element | null { } const state = useAppStore.getState() const order = state.tabBarOrderByWorktree[activeWorktreeId] ?? [] + const dirtyFileIds: string[] = [] for (const id of order) { if (id === tabId) { continue @@ -600,10 +757,9 @@ function Terminal(): React.JSX.Element | null { } else if ( state.openFiles.some((file) => file.worktreeId === activeWorktreeId && file.id === id) ) { - if ( - state.activeFileId === id && - state.openFiles.find((file) => file.id === id)?.isDirty - ) { + const file = state.openFiles.find((candidate) => candidate.id === id) + if (file?.isDirty) { + dirtyFileIds.push(id) continue } closeFile(id) @@ -614,8 +770,11 @@ function Terminal(): React.JSX.Element | null { closeBrowserTab(id) } } + if (dirtyFileIds.length > 0) { + queueEditorCloseRequests(dirtyFileIds) + } }, - [activeWorktreeId, closeBrowserTab, closeFile, closeTab] + [activeWorktreeId, closeBrowserTab, closeFile, closeTab, queueEditorCloseRequests] ) const handleCloseTabsToRight = useCallback( @@ -630,12 +789,18 @@ function Terminal(): React.JSX.Element | null { return } const rightIds = currentOrder.slice(index + 1) + const dirtyFileIds: string[] = [] for (const id of rightIds) { if ((state.tabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id)) { closeTab(id) } else if ( state.openFiles.some((file) => file.worktreeId === activeWorktreeId && file.id === id) ) { + const file = state.openFiles.find((candidate) => candidate.id === id) + if (file?.isDirty) { + dirtyFileIds.push(id) + continue + } closeFile(id) } else if ( (state.browserTabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id) @@ -644,10 +809,30 @@ function Terminal(): React.JSX.Element | null { closeBrowserTab(id) } } + if (dirtyFileIds.length > 0) { + queueEditorCloseRequests(dirtyFileIds) + } }, - [activeWorktreeId, closeBrowserTab, closeFile, closeTab] + [activeWorktreeId, closeBrowserTab, closeFile, closeTab, queueEditorCloseRequests] ) + const handleCloseAllFiles = useCallback(() => { + if (!activeWorktreeId) { + return + } + const state = useAppStore.getState() + const filesInWorktree = state.openFiles.filter((file) => file.worktreeId === activeWorktreeId) + const dirtyFileIds = filesInWorktree.filter((file) => file.isDirty).map((file) => file.id) + for (const file of filesInWorktree) { + if (!file.isDirty) { + closeFile(file.id) + } + } + if (dirtyFileIds.length > 0) { + queueEditorCloseRequests(dirtyFileIds) + } + }, [activeWorktreeId, closeFile, queueEditorCloseRequests]) + const handleActivateTab = useCallback( (tabId: string) => { setActiveTab(tabId) @@ -868,8 +1053,10 @@ function Terminal(): React.JSX.Element | null { const dirtyFiles = useAppStore.getState().openFiles.filter((f) => f.isDirty) if (dirtyFiles.length > 0) { - windowCloseAfterDirtyRef.current = { isQuitting } - setSaveDialogFileId(dirtyFiles[0].id) + queueEditorCloseRequests( + dirtyFiles.map((file) => file.id), + { isQuitting } + ) return } @@ -902,7 +1089,7 @@ function Terminal(): React.JSX.Element | null { } ) }) - }, []) + }, [queueEditorCloseRequests]) // Why: removeWorktree cleans up browser tab state in the store but cannot // call destroyPersistentWebview (renderer-only DOM code). This subscriber @@ -1005,7 +1192,7 @@ function Terminal(): React.JSX.Element | null { onActivateBrowserTab={handleActivateBrowserTab} onCloseBrowserTab={handleCloseBrowserTab} onDuplicateBrowserTab={handleDuplicateBrowserTab} - onCloseAllFiles={closeAllFiles} + onCloseAllFiles={handleCloseAllFiles} onPinFile={pinFile} tabBarOrder={tabBarOrder} />, @@ -1189,7 +1376,7 @@ function Terminal(): React.JSX.Element | null { Unsaved Changes {saveDialogFile - ? `"${saveDialogFile.relativePath.split('/').pop()}" has unsaved changes. Do you want to save before closing?` + ? `"${basename(saveDialogFile.relativePath)}" has unsaved changes. Do you want to save before closing?` : 'This file has unsaved changes.'} diff --git a/src/renderer/src/components/editor/editor-autosave.test.ts b/src/renderer/src/components/editor/editor-autosave.test.ts index 9484e7206c3..1157794b5df 100644 --- a/src/renderer/src/components/editor/editor-autosave.test.ts +++ b/src/renderer/src/components/editor/editor-autosave.test.ts @@ -4,7 +4,9 @@ import { canAutoSaveOpenFile, getOpenFilesForExternalFileChange, normalizeAutoSaveDelayMs, + ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, ORCA_EDITOR_QUIESCE_FILE_SAVES_EVENT, + requestEditorFileClose, requestEditorFileSave, requestEditorSaveQuiesce } from './editor-autosave' @@ -140,6 +142,21 @@ describe('requestEditorFileSave', () => { }) }) +describe('requestEditorFileClose', () => { + it('dispatches a close request event with the file id', () => { + const listener = vi.fn() + window.addEventListener(ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, listener as EventListener) + try { + requestEditorFileClose('file-1') + expect(listener).toHaveBeenCalledTimes(1) + const event = listener.mock.calls[0][0] as CustomEvent<{ fileId: string }> + expect(event.detail).toEqual({ fileId: 'file-1' }) + } finally { + window.removeEventListener(ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, listener as EventListener) + } + }) +}) + describe('getOpenFilesForExternalFileChange', () => { it('matches edit tabs and unstaged diff tabs for the same worktree file', () => { const matchingEdit = makeOpenFile() diff --git a/src/renderer/src/components/editor/editor-autosave.ts b/src/renderer/src/components/editor/editor-autosave.ts index 9550de9679f..6985cf2ad8f 100644 --- a/src/renderer/src/components/editor/editor-autosave.ts +++ b/src/renderer/src/components/editor/editor-autosave.ts @@ -13,6 +13,7 @@ export const ORCA_EDITOR_SAVE_FILE_EVENT = 'orca:editor-save-file' export const ORCA_EDITOR_SAVE_AND_CLOSE_EVENT = 'orca:save-and-close' export const ORCA_EDITOR_FILE_SAVED_EVENT = 'orca:editor-file-saved' export const ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT = 'orca:editor-request-cmd-save' +export const ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT = 'orca:editor-request-file-close' export type EditorPathMutationTarget = { worktreeId: string @@ -43,6 +44,10 @@ export type EditorFileSavedDetail = { content: string } +export type EditorRequestFileCloseDetail = { + fileId: string +} + export function canAutoSaveOpenFile(file: OpenFile): boolean { // Why: single-file editors and one-file unstaged diffs have an unambiguous // write target. Combined diff and conflict-review tabs can represent multiple @@ -131,6 +136,14 @@ export async function requestEditorFileSave(target: EditorSaveFileTarget): Promi }) } +export function requestEditorFileClose(fileId: string): void { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, { + detail: { fileId } + }) + ) +} + export function notifyEditorExternalFileChange(target: EditorPathMutationTarget): void { window.dispatchEvent( new CustomEvent(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, { diff --git a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts index 7c7a660a97b..19685a15839 100644 --- a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts +++ b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts @@ -17,6 +17,7 @@ import { createUntitledMarkdownFile } from '../../lib/create-untitled-markdown' import { getConnectionId } from '../../lib/connection-context' import { extractIpcErrorMessage } from '../../lib/ipc-error' import { destroyPersistentWebview } from '../browser-pane/BrowserPane' +import { requestEditorFileClose } from '../editor/editor-autosave' import { focusTerminalTabSurface } from '../../lib/focus-terminal-tab-surface' export type GroupEditorItem = OpenFile & { tabId: string } @@ -66,8 +67,6 @@ export function useTabGroupWorkspaceModel({ const focusGroup = useAppStore((state) => state.focusGroup) const activateTab = useAppStore((state) => state.activateTab) const closeUnifiedTab = useAppStore((state) => state.closeUnifiedTab) - const closeOtherTabs = useAppStore((state) => state.closeOtherTabs) - const closeTabsToRight = useAppStore((state) => state.closeTabsToRight) const closeEmptyGroup = useAppStore((state) => state.closeEmptyGroup) const createTab = useAppStore((state) => state.createTab) const closeTab = useAppStore((state) => state.closeTab) @@ -165,8 +164,17 @@ export function useTabGroupWorkspaceModel({ item.contentType === 'conflict-review') ) if (!otherReference) { + const file = useAppStore.getState().openFiles.find((candidate) => candidate.id === entityId) + if (file?.isDirty) { + // Why: split-group close actions bypass Terminal.tsx, but the unsaved + // confirmation + save/discard ordering must stay centralized there so + // tab close, bulk close, and window quit share one queueing flow. + requestEditorFileClose(entityId) + return false + } closeFile(entityId) } + return true }, [closeFile, worktreeId] ) @@ -199,7 +207,10 @@ export function useTabGroupWorkspaceModel({ destroyPersistentWebview(item.entityId) closeBrowserTab(item.entityId) } else { - closeEditorIfUnreferenced(item.entityId, item.id) + const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) + if (!canCloseTab) { + return + } closeUnifiedTab(item.id) } if (!opts?.skipEmptyCheck) { @@ -229,11 +240,14 @@ export function useTabGroupWorkspaceModel({ destroyPersistentWebview(item.entityId) closeBrowserTab(item.entityId) } else { - closeEditorIfUnreferenced(item.entityId, item.id) + const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) + if (canCloseTab) { + closeUnifiedTab(item.id) + } } } }, - [closeBrowserTab, closeEditorIfUnreferenced, closeTab, groupTabs] + [closeBrowserTab, closeEditorIfUnreferenced, closeTab, closeUnifiedTab, groupTabs] ) const activateTerminal = useCallback( @@ -360,6 +374,47 @@ export function useTabGroupWorkspaceModel({ } }, [closeItem, groupTabs]) + const closeOthers = useCallback( + (itemId: string) => { + const item = groupTabs.find((candidate) => candidate.id === itemId) + if (!item) { + return + } + // Why: the store's closeOtherTabs helper unconditionally closes every non-pinned + // sibling unified tab, including dirty editor tabs — stranding those files in + // openFiles without a tab if the user cancels the save dialog. Collect the target + // ids here instead and route them through the same dirty-aware closeMany path + // used by individual tab closes so the Cancel -> zombie-file hazard is impossible. + const siblingIds = groupTabs + .filter((candidate) => candidate.id !== itemId && !candidate.isPinned) + .map((candidate) => candidate.id) + closeMany(siblingIds) + }, + [closeMany, groupTabs] + ) + + const closeToRight = useCallback( + (itemId: string) => { + // Why: see closeOthers — the store's closeTabsToRight helper pre-closes dirty + // editor tabs before the save dialog resolves. Walking the group's tabOrder + // locally (unifiedTabsByWorktree is append-ordered, not visually ordered, so + // tabOrder is the canonical left-to-right sequence) and routing through + // closeMany keeps the dirty-aware flow intact. + const order = group?.tabOrder ?? [] + const index = order.indexOf(itemId) + if (index === -1) { + return + } + const tabById = new Map(groupTabs.map((candidate) => [candidate.id, candidate])) + const rightIds = order.slice(index + 1).filter((id) => { + const candidate = tabById.get(id) + return candidate ? !candidate.isPinned : false + }) + closeMany(rightIds) + }, + [closeMany, group, groupTabs] + ) + const tabBarOrder = useMemo( () => (group?.tabOrder ?? []).map((itemId) => { @@ -395,8 +450,8 @@ export function useTabGroupWorkspaceModel({ closeAllEditorTabsInGroup, closeGroup, closeItem, - closeOthers: (itemId: string) => closeMany(closeOtherTabs(itemId)), - closeToRight: (itemId: string) => closeMany(closeTabsToRight(itemId)), + closeOthers, + closeToRight, consumeSuppressedPtyExit, createSplitGroup, newBrowserTab: () => { diff --git a/src/renderer/src/components/terminal/unsaved-close-queue.test.ts b/src/renderer/src/components/terminal/unsaved-close-queue.test.ts new file mode 100644 index 00000000000..ae2be0e00c2 --- /dev/null +++ b/src/renderer/src/components/terminal/unsaved-close-queue.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest' +import { appendUniqueOpenFileIds } from './unsaved-close-queue' + +describe('appendUniqueOpenFileIds', () => { + it('appends only open file ids and skips duplicates', () => { + const result = appendUniqueOpenFileIds( + ['a'], + ['a', 'b', 'missing', 'c', 'b'], + new Set(['a', 'b', 'c']) + ) + expect(result).toEqual(['a', 'b', 'c']) + }) + + it('returns the original queue when no requested ids are provided', () => { + const queue = ['a'] + expect(appendUniqueOpenFileIds(queue, [], new Set(['a']))).toEqual(['a']) + }) +}) diff --git a/src/renderer/src/components/terminal/unsaved-close-queue.ts b/src/renderer/src/components/terminal/unsaved-close-queue.ts new file mode 100644 index 00000000000..8825b4f71b9 --- /dev/null +++ b/src/renderer/src/components/terminal/unsaved-close-queue.ts @@ -0,0 +1,20 @@ +export function appendUniqueOpenFileIds( + queue: string[], + requestedFileIds: string[], + openFileIds: ReadonlySet +): string[] { + if (requestedFileIds.length === 0) { + return queue + } + const nextQueue = [...queue] + for (const fileId of requestedFileIds) { + if (!openFileIds.has(fileId)) { + continue + } + if (nextQueue.includes(fileId)) { + continue + } + nextQueue.push(fileId) + } + return nextQueue +}