diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index dc39c3a0644..a8938a4e507 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -30,19 +30,14 @@ import { ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, ORCA_EDITOR_SAVE_AND_CLOSE_EVENT, ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, + type EditorRequestCmdSaveDetail, type EditorRequestFileCloseDetail, requestEditorSaveQuiesce } from './editor/editor-autosave' import { isIntentionalAppRestartInProgress } from '@/lib/updater-beforeunload' import { preventUnloadAndScheduleShutdownCheckpointReset } from '@/lib/shutdown-checkpoint-guard' import EditorAutosaveController from './editor/EditorAutosaveController' -import type { - Tab, - TabContentType, - TabGroupLayoutNode, - TerminalTab, - TuiAgent -} from '../../../shared/types' +import type { Tab, TabGroupLayoutNode, TerminalTab, TuiAgent } from '../../../shared/types' import { hasFeatureInteraction } from '../../../shared/feature-interactions' import BrowserPane from './browser-pane/BrowserPane' import { RetainedBrowserPaneOverlayLayer } from './browser-pane/BrowserPaneOverlayLayer' @@ -185,18 +180,12 @@ import { combineTerminalWorktreeParkIds, useManualTerminalWorktreeParking } from './terminal-pane/use-manual-terminal-worktree-parking' +import { EDITOR_TAB_CONTENT_TYPES, getEditorCmdSaveFileId } from './editor/editor-cmd-save-target' const EditorPanel = lazy(() => import('./editor/EditorPanel')) // Why: gate handler runs after a dialog advances so a stray carry-over click can't act on the next dialog; ~200ms absorbs a physical double-click while staying responsive. const CLOSE_DIALOG_DEBOUNCE_MS = 200 -const EDITOR_TAB_CONTENT_TYPES = new Set([ - 'editor', - 'diff', - 'conflict-review', - 'check-details' -]) - type TerminalStoreSnapshot = ReturnType function haveSameIdSet(left: ReadonlySet, right: ReadonlySet): boolean { @@ -2077,10 +2066,17 @@ function Terminal(): React.JSX.Element | null { target?.closest('textarea:not(.xterm-helper-textarea), input') !== null if (!inEditor) { const state = useAppStore.getState() - if (state.activeTabType === 'editor' && state.activeFileId) { + const floatingPanelOwnsEvent = + isEventTargetInsideFloatingWorkspacePanel(e.target) || floatingWorkspaceFocused + const requestedFileId = getEditorCmdSaveFileId(state, floatingPanelOwnsEvent) + if (requestedFileId) { e.preventDefault() notifyTerminalCapture('editor.save') - window.dispatchEvent(new Event(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT)) + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, { + detail: { fileId: requestedFileId } + }) + ) return } } diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx index 2e07c10efb4..233d8a9e123 100644 --- a/src/renderer/src/components/editor/EditorPanel.tsx +++ b/src/renderer/src/components/editor/EditorPanel.tsx @@ -23,16 +23,20 @@ import { selectEditorPanelGitStatusEntries } from './editor-panel-git-entry-selector' import { createEditorPanelDraftSelector } from './editor-panel-draft-selector' -import { attemptEditorFileSave } from './editor-file-save-attempt' import { createCurrentMarkdownArtifactRequest } from './markdown-artifact-upload' +import { useEditorPanelSave } from './useEditorPanelSave' function EditorPanelInner({ activeFileId: activeFileIdProp, activeViewStateId: activeViewStateIdProp, + isVisible = true, + isCmdSaveOwner = isVisible, markdownAnnotationsEnabled = true }: { activeFileId?: string | null activeViewStateId?: string | null + isVisible?: boolean + isCmdSaveOwner?: boolean markdownAnnotationsEnabled?: boolean } = {}): React.JSX.Element | null { const openFiles = useAppStore((s) => s.openFiles) @@ -114,7 +118,8 @@ function EditorPanelInner({ isChangesMode: requestedChangesMode, openFiles, gitStatusEntries, - editorViewMode + editorViewMode, + isVisible }) const isChangesMode = requestedChangesMode && @@ -172,37 +177,18 @@ function EditorPanelInner({ [activeFile, markFileDirty] ) - const handleSaveForFile = useCallback( - async (file: typeof activeFile, content: string): Promise => { - if (!file) { - return false - } - const saveTargetFile = - file.mode === 'markdown-preview' - ? (openFiles.find( - (openFile) => - openFile.id === file.markdownPreviewSourceFileId && openFile.mode === 'edit' - ) ?? null) - : file - if (!saveTargetFile) { - return false - } - if (saveTargetFile.isUntitled) { - requestRenameForFile(saveTargetFile.id) - return false - } - return attemptEditorFileSave({ fileId: saveTargetFile.id, fallbackContent: content }) - }, - [openFiles, requestRenameForFile] - ) - - const handleSave = useCallback( - async (content: string): Promise => { - return handleSaveForFile(activeFile, content) - }, - [activeFile, handleSaveForFile] - ) - useEditorCmdSaveRequest({ activeFile, openFiles, fileContents, handleSave }) + const { handleSave, handleSaveForFile } = useEditorPanelSave({ + activeFile, + openFiles, + requestRenameForFile + }) + useEditorCmdSaveRequest({ + activeFile, + openFiles, + fileContents, + handleSave, + enabled: isCmdSaveOwner + }) const handleCopyPath = useCallback(async (): Promise => { if (!activeFile) { diff --git a/src/renderer/src/components/editor/editor-autosave.ts b/src/renderer/src/components/editor/editor-autosave.ts index efeda658724..0d33a9c1871 100644 --- a/src/renderer/src/components/editor/editor-autosave.ts +++ b/src/renderer/src/components/editor/editor-autosave.ts @@ -49,6 +49,10 @@ export type EditorRequestFileCloseDetail = { fileId: string } +export type EditorRequestCmdSaveDetail = { + fileId: string +} + export function isExternalReloadableEditorTab(file: OpenFile): boolean { return ( file.mode === 'edit' || diff --git a/src/renderer/src/components/editor/editor-cmd-save-target.test.ts b/src/renderer/src/components/editor/editor-cmd-save-target.test.ts new file mode 100644 index 00000000000..518f8c8b859 --- /dev/null +++ b/src/renderer/src/components/editor/editor-cmd-save-target.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it, vi } from 'vitest' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import type { Tab } from '../../../../shared/types' +import { getEditorCmdSaveFileId } from './editor-cmd-save-target' + +function makeTab(contentType: Tab['contentType'], entityId: string): Tab { + return { + id: `tab-${entityId}`, + entityId, + contentType, + label: entityId, + groupId: 'group-1', + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +describe('getEditorCmdSaveFileId', () => { + it('targets the main active editor when the floating panel does not own the event', () => { + const getActiveTab = vi.fn(() => makeTab('editor', 'floating-file')) + + expect( + getEditorCmdSaveFileId( + { + activeFileId: 'main-file', + activeTabType: 'editor', + activeView: 'terminal', + getActiveTab + }, + false + ) + ).toBe('main-file') + expect(getActiveTab).not.toHaveBeenCalled() + }) + + it('claims nothing on a non-workspace view so the shortcut is not swallowed', () => { + const getActiveTab = vi.fn(() => null) + + for (const activeView of ['tasks', 'settings', 'activity'] as const) { + expect( + getEditorCmdSaveFileId( + { activeFileId: 'main-file', activeTabType: 'editor', activeView, getActiveTab }, + false + ) + ).toBeNull() + } + }) + + it('targets only an active floating editor and never falls through to main', () => { + const getActiveTab = vi + .fn<(worktreeId: string) => Tab | null>() + .mockReturnValueOnce(makeTab('editor', 'floating-file')) + .mockReturnValueOnce(makeTab('browser', 'floating-browser')) + const state = { + activeFileId: 'main-file', + activeTabType: 'editor', + activeView: 'terminal' as const, + getActiveTab + } + + expect(getEditorCmdSaveFileId(state, true)).toBe('floating-file') + expect(getEditorCmdSaveFileId(state, true)).toBeNull() + expect(getActiveTab).toHaveBeenNthCalledWith(1, FLOATING_TERMINAL_WORKTREE_ID) + expect(getActiveTab).toHaveBeenNthCalledWith(2, FLOATING_TERMINAL_WORKTREE_ID) + }) + + it('still targets the floating editor from a non-workspace view', () => { + const getActiveTab = vi.fn(() => makeTab('editor', 'floating-file')) + + expect( + getEditorCmdSaveFileId( + { activeFileId: 'main-file', activeTabType: 'editor', activeView: 'tasks', getActiveTab }, + true + ) + ).toBe('floating-file') + }) +}) diff --git a/src/renderer/src/components/editor/editor-cmd-save-target.ts b/src/renderer/src/components/editor/editor-cmd-save-target.ts new file mode 100644 index 00000000000..0d021a87790 --- /dev/null +++ b/src/renderer/src/components/editor/editor-cmd-save-target.ts @@ -0,0 +1,34 @@ +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import type { Tab, TabContentType, TopLevelView } from '../../../../shared/types' + +export const EDITOR_TAB_CONTENT_TYPES = new Set([ + 'editor', + 'diff', + 'conflict-review', + 'check-details' +]) + +type EditorCmdSaveState = { + activeFileId: string | null + activeTabType: string | null + activeView: TopLevelView + getActiveTab: (worktreeId: string) => Tab | null +} + +export function getEditorCmdSaveFileId( + state: EditorCmdSaveState, + floatingPanelOwnsEvent: boolean +): string | null { + if (!floatingPanelOwnsEvent) { + // Why: outside the workspace view no mounted panel claims the request, so + // returning an id would swallow Cmd/Ctrl+S on Tasks/Settings without saving. + // The floating panel floats above every view and keeps its own ownership. + return state.activeView === 'terminal' && state.activeTabType === 'editor' + ? state.activeFileId + : null + } + const activeTab = state.getActiveTab(FLOATING_TERMINAL_WORKTREE_ID) + return activeTab && EDITOR_TAB_CONTENT_TYPES.has(activeTab.contentType) + ? activeTab.entityId + : null +} diff --git a/src/renderer/src/components/editor/editor-panel-content-types.ts b/src/renderer/src/components/editor/editor-panel-content-types.ts index c44df176150..98591ffa8e6 100644 --- a/src/renderer/src/components/editor/editor-panel-content-types.ts +++ b/src/renderer/src/components/editor/editor-panel-content-types.ts @@ -25,6 +25,8 @@ export type FileContent = { mimeType?: string fileIdentity?: string loadError?: string + /** Superseded by an external change; still rendered until the lazy reload lands. */ + isStale?: boolean } export type DiffContent = GitDiffResult diff --git a/src/renderer/src/components/editor/editor-save-target.ts b/src/renderer/src/components/editor/editor-save-target.ts new file mode 100644 index 00000000000..d77c157b9fc --- /dev/null +++ b/src/renderer/src/components/editor/editor-save-target.ts @@ -0,0 +1,16 @@ +import type { OpenFile } from '@/store/slices/editor' + +export function getEditorSaveTargetFile( + activeFile: OpenFile, + openFiles: OpenFile[] +): OpenFile | null { + if (activeFile.mode !== 'markdown-preview') { + return activeFile + } + return ( + openFiles.find( + (openFile) => + openFile.id === activeFile.markdownPreviewSourceFileId && openFile.mode === 'edit' + ) ?? null + ) +} diff --git a/src/renderer/src/components/editor/useEditorCmdSaveRequest.test.tsx b/src/renderer/src/components/editor/useEditorCmdSaveRequest.test.tsx new file mode 100644 index 00000000000..ca27cf31126 --- /dev/null +++ b/src/renderer/src/components/editor/useEditorCmdSaveRequest.test.tsx @@ -0,0 +1,142 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT } from './editor-autosave' +import { useEditorCmdSaveRequest } from './useEditorCmdSaveRequest' + +const storeState = vi.hoisted(() => ({ editorDrafts: {} as Record })) +const EMPTY_FILE_CONTENTS: Parameters[0]['fileContents'] = {} + +vi.mock('@/store', () => ({ useAppStore: { getState: () => storeState } })) + +type ProbeProps = { + activeFile: OpenFile + enabled: boolean + fileContents?: Parameters[0]['fileContents'] + onSave: (content: string) => Promise + openFiles?: OpenFile[] +} + +function SaveProbe({ + activeFile, + enabled, + fileContents = EMPTY_FILE_CONTENTS, + onSave, + openFiles +}: ProbeProps): null { + useEditorCmdSaveRequest({ + activeFile, + openFiles: openFiles ?? [activeFile], + fileContents, + handleSave: onSave, + enabled + }) + return null +} + +function makeFile(id: string): OpenFile { + return { + id, + filePath: `/repo/${id}.md`, + relativePath: `${id}.md`, + worktreeId: `worktree-${id}`, + language: 'markdown', + isDirty: true, + mode: 'edit' + } +} + +describe('useEditorCmdSaveRequest', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + storeState.editorDrafts = {} + container = document.body.appendChild(document.createElement('div')) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + }) + + it('saves only the visible panel that owns the requested file', () => { + const visibleFile = makeFile('visible') + const otherVisibleFile = makeFile('other-visible') + const hiddenFile = makeFile('hidden') + const visibleSave = vi.fn(async () => true) + const mirroredSave = vi.fn(async () => true) + const otherVisibleSave = vi.fn(async () => true) + const hiddenSave = vi.fn(async () => true) + storeState.editorDrafts = { + [visibleFile.id]: 'visible draft', + [otherVisibleFile.id]: 'other draft', + [hiddenFile.id]: 'hidden draft' + } + + act(() => { + root.render( + <> + + + + + + ) + }) + act(() => { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, { + detail: { fileId: visibleFile.id } + }) + ) + }) + + expect(visibleSave).toHaveBeenCalledExactlyOnceWith('visible draft') + expect(mirroredSave).not.toHaveBeenCalled() + expect(otherVisibleSave).not.toHaveBeenCalled() + expect(hiddenSave).not.toHaveBeenCalled() + }) + + it('uses the preview tab for ownership and the source file for content', () => { + const sourceFile = makeFile('source') + const previewFile: OpenFile = { + ...sourceFile, + id: 'markdown-preview::source', + markdownPreviewSourceFileId: sourceFile.id, + mode: 'markdown-preview' + } + const save = vi.fn(async () => true) + storeState.editorDrafts = { [sourceFile.id]: 'source draft' } + + act(() => { + root.render( + + ) + }) + act(() => { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, { + detail: { fileId: sourceFile.id } + }) + ) + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, { + detail: { fileId: previewFile.id } + }) + ) + }) + + expect(save).toHaveBeenCalledExactlyOnceWith('source draft') + }) +}) diff --git a/src/renderer/src/components/editor/useEditorCmdSaveRequest.ts b/src/renderer/src/components/editor/useEditorCmdSaveRequest.ts index 41df5a64037..13ee33f09f4 100644 --- a/src/renderer/src/components/editor/useEditorCmdSaveRequest.ts +++ b/src/renderer/src/components/editor/useEditorCmdSaveRequest.ts @@ -1,11 +1,16 @@ import { useEffect } from 'react' import { useAppStore } from '@/store' import type { OpenFile } from '@/store/slices/editor' -import { ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT } from './editor-autosave' +import { + ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, + type EditorRequestCmdSaveDetail +} from './editor-autosave' import type { FileContent } from './editor-panel-content-types' +import { getEditorSaveTargetFile } from './editor-save-target' type UseEditorCmdSaveRequestParams = { activeFile: OpenFile | null + enabled: boolean openFiles: OpenFile[] fileContents: Record handleSave: (content: string) => Promise @@ -13,22 +18,23 @@ type UseEditorCmdSaveRequestParams = { export function useEditorCmdSaveRequest({ activeFile, + enabled, openFiles, fileContents, handleSave }: UseEditorCmdSaveRequestParams): void { useEffect(() => { - const handler = (): void => { - if (!activeFile) { + if (!enabled) { + return + } + const handler = (event: Event): void => { + if ( + !activeFile || + (event as CustomEvent).detail?.fileId !== activeFile.id + ) { return } - const saveTargetFile = - activeFile.mode === 'markdown-preview' - ? (openFiles.find( - (openFile) => - openFile.id === activeFile.markdownPreviewSourceFileId && openFile.mode === 'edit' - ) ?? null) - : activeFile + const saveTargetFile = getEditorSaveTargetFile(activeFile, openFiles) if (!saveTargetFile) { return } @@ -46,5 +52,5 @@ export function useEditorCmdSaveRequest({ } window.addEventListener(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, handler) return () => window.removeEventListener(ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, handler) - }, [activeFile, fileContents, handleSave, openFiles]) + }, [activeFile, enabled, fileContents, handleSave, openFiles]) } diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index f9cda5dc753..42ae831264e 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -1,7 +1,7 @@ /* oxlint-disable max-lines -- Why: content loading, retry, and external-change subscriptions share in-flight caches and state setters; splitting them would make the hook coordination harder to audit. */ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import type { OpenFile } from '@/store/slices/editor' import { getConnectionIdForFile, isWorktreeConnectionResolved } from '@/lib/connection-context' import { joinPath } from '@/lib/path' @@ -32,6 +32,7 @@ import { shouldReloadDiffOnGitStatusChange } from './editor-panel-diff-reload' import { + type EditorPanelContentLoadOptions, useEditorPanelExternalContentEvents, usePruneClosedEditorContent } from './useEditorPanelExternalContentEvents' @@ -39,14 +40,20 @@ import { useEditorPanelFileLoadRetry } from './useEditorPanelFileLoadRetry' import { useLocalLogTail } from './useLocalLogTail' import { migrateRestoredEditorFileOwner } from './migrate-restored-editor-file-owner' -const inFlightFileReads = new Map>() -const inFlightDiffReads = new Map>() +type InFlightContentRead = { + externalEventGeneration?: number + promise: Promise +} + +const inFlightFileReads = new Map>() +const inFlightDiffReads = new Map>() type GitStatusByWorktree = ReturnType['gitStatusByWorktree'] type EditorViewModeByFile = ReturnType['editorViewMode'] type UseEditorPanelContentStateParams = { activeFile: OpenFile | null + isVisible?: boolean isChangesMode: boolean openFiles: OpenFile[] gitStatusEntries: GitStatusByWorktree[string] | undefined @@ -78,6 +85,18 @@ function stampCleanTabDiskBaseline(id: string, result: FileContent): void { } } +// Why: the newest read for a tab is already on its way, so a re-run of the lazy +// effect (a worktree flip-flop re-adds `isVisible`) must not fire a second RPC. +// An invalidation bumps the tab's generation, so a superseded read never counts. +function hasLiveRead( + generationsById: Record, + outstandingById: Record, + id: string +): boolean { + const generation = generationsById[id] + return generation !== undefined && outstandingById[id] === generation +} + function inFlightReadKey(connectionId: string | undefined, filePath: string): string { return `${connectionId ?? ''}::${filePath}` } @@ -100,6 +119,7 @@ function inFlightDiffKey( export function useEditorPanelContentState({ activeFile, + isVisible = true, isChangesMode, openFiles, gitStatusEntries, @@ -108,7 +128,6 @@ export function useEditorPanelContentState({ const [fileContents, setFileContents] = useState>({}) const [diffContents, setDiffContents] = useState>({}) const diffContentsRef = useRef(diffContents) - diffContentsRef.current = diffContents const fileLoadRetryAttemptsRef = useRef>({}) // Why: per-tab read generations let a forced/external reload supersede an // older in-flight read so a slower stale promise cannot overwrite fresh state. @@ -116,14 +135,74 @@ export function useEditorPanelContentState({ const diffReadGenerationRef = useRef>({}) const fileReadGenerationCounterRef = useRef(0) const diffReadGenerationCounterRef = useRef(0) + const outstandingFileReadsRef = useRef>({}) + const outstandingDiffReadsRef = useRef>({}) const openFilesRef = useRef(openFiles) - openFilesRef.current = openFiles const editorViewModeRef = useRef(editorViewMode) - editorViewModeRef.current = editorViewMode + const isVisibleRef = useRef(isVisible) const selectedConflictReviewFile = activeFile?.mode === 'conflict-review' && activeFile.conflictReview?.selectedFileId ? (openFiles.find((file) => file.id === activeFile.conflictReview?.selectedFileId) ?? null) : null + const activeContentFileId = selectedConflictReviewFile?.id ?? activeFile?.id ?? null + const activeContentFileIdRef = useRef(activeContentFileId) + + useLayoutEffect(() => { + // Why: event-driven readers must only observe state from committed renders. + diffContentsRef.current = diffContents + openFilesRef.current = openFiles + editorViewModeRef.current = editorViewMode + isVisibleRef.current = isVisible + activeContentFileIdRef.current = activeContentFileId + }, [activeContentFileId, diffContents, editorViewMode, isVisible, openFiles]) + + const invalidateFileContent = useCallback((fileIds: string[]): void => { + const uniqueIds = new Set(fileIds) + for (const fileId of uniqueIds) { + fileReadGenerationRef.current[fileId] = ++fileReadGenerationCounterRef.current + delete fileLoadRetryAttemptsRef.current[fileId] + } + setFileContents((prev) => { + const next = { ...prev } + let changed = false + for (const fileId of uniqueIds) { + const existing = next[fileId] + // Why: keep the last-known bytes rendered and swap them when the lazy + // reload lands — dropping them flashes "Loading…" on every reveal. + if (existing && existing.isStale !== true) { + next[fileId] = { ...existing, isStale: true } + changed = true + } + } + return changed ? next : prev + }) + }, []) + + const invalidateDiffContent = useCallback((fileIds: string[]): void => { + const uniqueIds = new Set(fileIds) + for (const fileId of uniqueIds) { + diffReadGenerationRef.current[fileId] = ++diffReadGenerationCounterRef.current + } + setDiffContents((prev) => { + const next = { ...prev } + let changed = false + for (const fileId of uniqueIds) { + if (fileId in next) { + delete next[fileId] + changed = true + } + } + return changed ? next : prev + }) + }, []) + + const invalidateContent = useCallback( + (fileIds: string[]): void => { + invalidateFileContent(fileIds) + invalidateDiffContent(fileIds) + }, + [invalidateDiffContent, invalidateFileContent] + ) const loadFileContent = useCallback( async ( @@ -131,11 +210,12 @@ export function useEditorPanelContentState({ id: string, worktreeId?: string, relativePath?: string, - options?: { force?: boolean } + options?: EditorPanelContentLoadOptions ): Promise => { const generation = fileReadGenerationCounterRef.current + 1 fileReadGenerationCounterRef.current = generation fileReadGenerationRef.current[id] = generation + outstandingFileReadsRef.current[id] = generation try { const resolvedConnectionId = getConnectionIdForFile(worktreeId ?? null, filePath) const connectionId = resolvedConnectionId ?? undefined @@ -219,14 +299,19 @@ export function useEditorPanelContentState({ } const readScope = getRuntimeFileReadScope(readSettings, readConnectionId) const key = inFlightReadKey(readScope, filePath) - if (options?.force) { + const registeredRead = inFlightFileReads.get(key) + if ( + options?.force && + (options.externalEventGeneration === undefined || + registeredRead?.externalEventGeneration !== options.externalEventGeneration) + ) { // Why: forced reloads must not attach to a currently registered read // started before the external change landed. inFlightFileReads.delete(key) } let pending = inFlightFileReads.get(key) if (!pending) { - pending = readRuntimeFileContent({ + const promise = readRuntimeFileContent({ settings: readSettings, filePath, relativePath: readRelativePath, @@ -235,6 +320,7 @@ export function useEditorPanelContentState({ expectedExternalSshTargetId: restoredOpenFile?.externalSshTargetId, includeLocalLogMetadata: isLiveTailLogTab }) as Promise + pending = { externalEventGeneration: options?.externalEventGeneration, promise } inFlightFileReads.set(key, pending) queueMicrotask(() => { if (inFlightFileReads.get(key) === pending) { @@ -242,7 +328,7 @@ export function useEditorPanelContentState({ } }) } - const result = await pending + const result = await pending.promise if (fileReadGenerationRef.current[id] !== generation) { return } @@ -258,19 +344,24 @@ export function useEditorPanelContentState({ ...prev, [id]: { content: '', isBinary: false, loadError: message } })) + } finally { + if (outstandingFileReadsRef.current[id] === generation) { + delete outstandingFileReadsRef.current[id] + } } }, [] ) const loadDiffContent = useCallback( - async (file: OpenFile | null, options?: { force?: boolean }): Promise => { + async (file: OpenFile | null, options?: EditorPanelContentLoadOptions): Promise => { if (!file || (file.mode === 'edit' && !canUseChangesModeForFile(file))) { return } const generation = diffReadGenerationCounterRef.current + 1 diffReadGenerationCounterRef.current = generation diffReadGenerationRef.current[file.id] = generation + outstandingDiffReadsRef.current[file.id] = generation try { const worktreePath = file.filePath.slice( 0, @@ -293,14 +384,19 @@ export function useEditorPanelContentState({ gitScope ?? undefined, compareAgainstHead ) - if (options?.force) { + const registeredRead = inFlightDiffReads.get(key) + if ( + options?.force && + (options.externalEventGeneration === undefined || + registeredRead?.externalEventGeneration !== options.externalEventGeneration) + ) { // Why: forced diff reloads must not attach to a read started before // the external change landed. inFlightDiffReads.delete(key) } let pending = inFlightDiffReads.get(key) if (!pending) { - pending = ( + const promise = ( effectiveDiffSource === 'commit' ? commitCompare ? getRuntimeGitCommitDiff( @@ -351,6 +447,7 @@ export function useEditorPanelContentState({ } ) ) as Promise + pending = { externalEventGeneration: options?.externalEventGeneration, promise } inFlightDiffReads.set(key, pending) queueMicrotask(() => { if (inFlightDiffReads.get(key) === pending) { @@ -358,7 +455,7 @@ export function useEditorPanelContentState({ } }) } - const result = await pending + const result = await pending.promise if (diffReadGenerationRef.current[file.id] !== generation) { return } @@ -377,6 +474,10 @@ export function useEditorPanelContentState({ modifiedIsBinary: false } })) + } finally { + if (outstandingDiffReadsRef.current[file.id] === generation) { + delete outstandingDiffReadsRef.current[file.id] + } } }, [] @@ -417,7 +518,21 @@ export function useEditorPanelContentState({ useLocalLogTail({ openFiles, fileContents, setFileContents, reloadContent }) + const needsFileRead = (fileId: string): boolean => { + const cached = fileContents[fileId] + return ( + (!cached || cached.isStale === true) && + !hasLiveRead(fileReadGenerationRef.current, outstandingFileReadsRef.current, fileId) + ) + } + const needsDiffRead = (fileId: string): boolean => + !diffContents[fileId] && + !hasLiveRead(diffReadGenerationRef.current, outstandingDiffReadsRef.current, fileId) + useEffect(() => { + if (!isVisible) { + return + } if (activeFile?.mode === 'conflict-review' && !selectedConflictReviewFile) { const snapshotEntries = activeFile.conflictReview?.entries ?? [] if (snapshotEntries.length === 0) { @@ -437,7 +552,7 @@ export function useEditorPanelContentState({ } const absolutePath = joinPath(activeFile.filePath, entry.path) - if (!fileContents[absolutePath]) { + if (needsFileRead(absolutePath)) { void loadFileContent(absolutePath, absolutePath, activeFile.worktreeId, entry.path) } } @@ -452,7 +567,7 @@ export function useEditorPanelContentState({ if (fileToLoad.conflict?.kind === 'conflict-placeholder') { return } - if (!fileContents[fileToLoad.id]) { + if (needsFileRead(fileToLoad.id)) { void loadFileContent( fileToLoad.filePath, fileToLoad.id, @@ -460,10 +575,10 @@ export function useEditorPanelContentState({ fileToLoad.relativePath ) } - if (isChangesMode && !diffContents[fileToLoad.id]) { + if (isChangesMode && needsDiffRead(fileToLoad.id)) { void loadDiffContent(fileToLoad) } - } else if (isReloadableSingleFileDiffTab(fileToLoad) && !diffContents[fileToLoad.id]) { + } else if (isReloadableSingleFileDiffTab(fileToLoad) && needsDiffRead(fileToLoad.id)) { void loadDiffContent(fileToLoad) } // oxlint-disable-next-line react-hooks/exhaustive-deps @@ -474,11 +589,12 @@ export function useEditorPanelContentState({ activeFile?.conflictReview?.snapshotTimestamp, selectedConflictReviewFile?.id, isChangesMode, + isVisible, gitStatusEntries ]) useEditorPanelFileLoadRetry({ - activeFile, + activeFile: isVisible ? activeFile : null, fileContents, fileLoadRetryAttemptsRef, loadFileContent, @@ -523,6 +639,10 @@ export function useEditorPanelContentState({ if (!(isChangesMode || activeFileShouldReloadOnGitStatusChange)) { return } + if (!isVisibleRef.current) { + invalidateDiffContent([current.id]) + return + } // Why: the lazy-load effect already fetches on first open; forcing here // races a duplicate git-diff RPC for the same tab. if (!diffContentsRef.current[current.id]) { @@ -534,6 +654,7 @@ export function useEditorPanelContentState({ activeFileGitStatusSignature, isChangesMode, activeFile?.id, + invalidateDiffContent, loadDiffContent ]) @@ -546,16 +667,12 @@ export function useEditorPanelContentState({ if (!current || !isReloadableSingleFileDiffTab(current)) { return } - setDiffContents((prev) => { - if (!prev[current.id]) { - return prev - } - const next = { ...prev } - delete next[current.id] - return next - }) + invalidateDiffContent([current.id]) + if (!isVisibleRef.current) { + return + } void loadDiffContent(current, { force: true }) - }, [activeFile?.diffContentReloadNonce, activeFile?.id, loadDiffContent]) + }, [activeFile?.diffContentReloadNonce, activeFile?.id, invalidateDiffContent, loadDiffContent]) useEffect(() => { const nonce = activeFile?.fileContentReloadNonce @@ -570,20 +687,26 @@ export function useEditorPanelContentState({ ) { return } - setFileContents((prev) => { - if (!prev[current.id]) { - return prev - } - const next = { ...prev } - delete next[current.id] - return next - }) + invalidateFileContent([current.id]) + if (!isVisibleRef.current) { + return + } void loadFileContent(current.filePath, current.id, current.worktreeId, current.relativePath, { force: true }) - }, [activeFile?.fileContentReloadNonce, activeFile?.filePath, activeFile?.id, loadFileContent]) + }, [ + activeFile?.fileContentReloadNonce, + activeFile?.filePath, + activeFile?.id, + invalidateFileContent, + loadFileContent + ]) useEditorPanelExternalContentEvents({ + activeContentFileIdRef, + invalidateContent, + invalidateDiffContent, + isVisibleRef, loadDiffContent, loadFileContent, openFilesRef, diff --git a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.test.tsx b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.test.tsx new file mode 100644 index 00000000000..12f5471e54c --- /dev/null +++ b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.test.tsx @@ -0,0 +1,242 @@ +// @vitest-environment happy-dom + +import { act, useLayoutEffect, useRef, useState } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT } from './editor-autosave' +import type { DiffContent, FileContent } from './editor-panel-content-types' +import { useEditorPanelExternalContentEvents } from './useEditorPanelExternalContentEvents' + +type ProbeCalls = { + invalidate: ReturnType + invalidateDiff: ReturnType + loadDiff: ReturnType + loadFile: ReturnType +} + +type ProbeProps = { + activeFileId: string + calls: ProbeCalls + isVisible: boolean + openFiles: OpenFile[] +} + +function ExternalContentProbe({ activeFileId, calls, isVisible, openFiles }: ProbeProps): null { + const activeContentFileIdRef = useRef(activeFileId) + const isVisibleRef = useRef(isVisible) + const openFilesRef = useRef(openFiles) + const editorViewModeRef = useRef({}) + const [, setFileContents] = useState>({}) + const [, setDiffContents] = useState>({}) + + useLayoutEffect(() => { + activeContentFileIdRef.current = activeFileId + isVisibleRef.current = isVisible + openFilesRef.current = openFiles + }, [activeFileId, isVisible, openFiles]) + + useEditorPanelExternalContentEvents({ + activeContentFileIdRef, + editorViewModeRef, + invalidateContent: calls.invalidate, + invalidateDiffContent: calls.invalidateDiff, + isVisibleRef, + loadDiffContent: calls.loadDiff, + loadFileContent: calls.loadFile, + openFilesRef, + setDiffContents, + setFileContents + } as Parameters[0]) + return null +} + +function makeFile(id: string, overrides: Partial = {}): OpenFile { + return { + id, + filePath: `/remote/repo/${id}.ts`, + relativePath: `${id}.ts`, + worktreeId: 'ssh-worktree', + language: 'typescript', + isDirty: false, + mode: 'edit', + ...overrides + } +} + +function makeCalls(): ProbeCalls { + return { + invalidate: vi.fn(), + invalidateDiff: vi.fn(), + loadDiff: vi.fn(async () => undefined), + loadFile: vi.fn(async () => undefined) + } +} + +function dispatchExternalChange(relativePath: string): void { + act(() => { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, { + detail: { + worktreeId: 'ssh-worktree', + worktreePath: '/remote/repo', + relativePath + } + }) + ) + }) +} + +describe('useEditorPanelExternalContentEvents', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + container = document.body.appendChild(document.createElement('div')) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + }) + + it('routes one remote change to one visible owner instead of every retained panel', () => { + const changedFile = makeFile('changed') + const otherFiles = Array.from({ length: 9 }, (_, index) => makeFile(`other-${index}`)) + const openFiles = [changedFile, ...otherFiles] + const ownerCalls = makeCalls() + const retainedCalls = otherFiles.map(() => makeCalls()) + + act(() => { + root.render( + <> + + {otherFiles.map((file, index) => ( + + ))} + + ) + }) + dispatchExternalChange(changedFile.relativePath) + + expect(ownerCalls.loadFile).toHaveBeenCalledOnce() + expect(retainedCalls.flatMap((calls) => calls.loadFile.mock.calls)).toHaveLength(0) + for (const calls of retainedCalls) { + expect(calls.invalidate).toHaveBeenCalledExactlyOnceWith([changedFile.id]) + } + }) + + it('invalidates a hidden owner without reloading until reveal', () => { + const changedFile = makeFile('changed') + const calls = makeCalls() + + act(() => { + root.render( + + ) + }) + dispatchExternalChange(changedFile.relativePath) + + expect(calls.loadFile).not.toHaveBeenCalled() + expect(calls.invalidate).toHaveBeenCalledExactlyOnceWith([changedFile.id]) + }) + + it('keeps dirty hidden content intact', () => { + const changedFile = makeFile('changed', { isDirty: true }) + const calls = makeCalls() + + act(() => { + root.render( + + ) + }) + dispatchExternalChange(changedFile.relativePath) + + expect(calls.loadFile).not.toHaveBeenCalled() + expect(calls.invalidate).not.toHaveBeenCalled() + }) + + it('invalidates an inactive cached Changes diff after reloading its visible file', () => { + const changedFile = makeFile('changed') + const calls = makeCalls() + + act(() => { + root.render( + + ) + }) + dispatchExternalChange(changedFile.relativePath) + + expect(calls.loadFile).toHaveBeenCalledOnce() + expect(calls.loadDiff).not.toHaveBeenCalled() + expect(calls.invalidateDiff).toHaveBeenCalledExactlyOnceWith([changedFile.id]) + }) + + it('uses one event generation for visible source and preview reloads', () => { + const source = makeFile('source', { + filePath: '/remote/repo/shared.md', + relativePath: 'shared.md' + }) + const preview = makeFile('preview', { + filePath: '/remote/repo/shared.md', + relativePath: 'shared.md', + mode: 'markdown-preview', + markdownPreviewSourceFileId: source.id + }) + const sourceCalls = makeCalls() + const previewCalls = makeCalls() + + act(() => { + root.render( + <> + + + + ) + }) + dispatchExternalChange(source.relativePath) + + const sourceOptions = sourceCalls.loadFile.mock.calls[0]?.[4] + const previewOptions = previewCalls.loadFile.mock.calls[0]?.[4] + expect(sourceOptions?.externalEventGeneration).toBeTypeOf('number') + expect(previewOptions?.externalEventGeneration).toBe(sourceOptions?.externalEventGeneration) + }) +}) diff --git a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts index 19650b1cd64..7a5077b59bb 100644 --- a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts +++ b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts @@ -13,14 +13,23 @@ import { isReloadableSingleFileDiffTab } from './editor-panel-diff-reload' type EditorViewModeByFile = ReturnType['editorViewMode'] +export type EditorPanelContentLoadOptions = { + force?: boolean + externalEventGeneration?: number +} + type UseEditorPanelExternalContentEventsParams = { - loadDiffContent: (file: OpenFile | null, options?: { force?: boolean }) => Promise + activeContentFileIdRef: MutableRefObject + invalidateContent: (fileIds: string[]) => void + invalidateDiffContent: (fileIds: string[]) => void + isVisibleRef: MutableRefObject + loadDiffContent: (file: OpenFile | null, options?: EditorPanelContentLoadOptions) => Promise loadFileContent: ( filePath: string, id: string, worktreeId?: string, relativePath?: string, - options?: { force?: boolean } + options?: EditorPanelContentLoadOptions ) => Promise openFilesRef: MutableRefObject editorViewModeRef: MutableRefObject @@ -28,7 +37,24 @@ type UseEditorPanelExternalContentEventsParams = { setDiffContents: Dispatch>> } +const externalEventGenerations = new WeakMap() +let externalEventGenerationCounter = 0 + +function getExternalEventGeneration(event: Event): number { + const existing = externalEventGenerations.get(event) + if (existing !== undefined) { + return existing + } + const generation = ++externalEventGenerationCounter + externalEventGenerations.set(event, generation) + return generation +} + export function useEditorPanelExternalContentEvents({ + activeContentFileIdRef, + invalidateContent, + invalidateDiffContent, + isVisibleRef, loadDiffContent, loadFileContent, openFilesRef, @@ -42,6 +68,9 @@ export function useEditorPanelExternalContentEvents({ if (!detail) { return } + const eventGeneration = getExternalEventGeneration(event) + const invalidatedDiffFileIds: string[] = [] + const invalidatedFileIds: string[] = [] for (const file of getOpenFilesForExternalFileChange(openFilesRef.current, detail)) { // Why: a dirty file keeps its unsaved buffer (issue #7265) — it is // marked changed-on-disk upstream and resolves via the editor banner, @@ -49,24 +78,52 @@ export function useEditorPanelExternalContentEvents({ if (file.isDirty) { continue } + if (!isVisibleRef.current || file.id !== activeContentFileIdRef.current) { + invalidatedFileIds.push(file.id) + continue + } if (file.mode === 'edit' || file.mode === 'markdown-preview') { // Why: external writes must replace any in-flight pre-change read so // the tab shows the new on-disk content, not a stale dedupe result. void loadFileContent(file.filePath, file.id, file.worktreeId, file.relativePath, { - force: true + force: true, + externalEventGeneration: eventGeneration }) if (editorViewModeRef.current[file.id] === 'changes') { - void loadDiffContent(file, { force: true }) + void loadDiffContent(file, { + force: true, + externalEventGeneration: eventGeneration + }) + } else { + invalidatedDiffFileIds.push(file.id) } } else if (isReloadableSingleFileDiffTab(file)) { - void loadDiffContent(file, { force: true }) + void loadDiffContent(file, { + force: true, + externalEventGeneration: eventGeneration + }) } } + if (invalidatedFileIds.length > 0) { + invalidateContent(invalidatedFileIds) + } + if (invalidatedDiffFileIds.length > 0) { + invalidateDiffContent(invalidatedDiffFileIds) + } } window.addEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, handler as EventListener) return () => window.removeEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, handler as EventListener) - }, [editorViewModeRef, loadDiffContent, loadFileContent, openFilesRef]) + }, [ + activeContentFileIdRef, + editorViewModeRef, + invalidateContent, + invalidateDiffContent, + isVisibleRef, + loadDiffContent, + loadFileContent, + openFilesRef + ]) useEffect(() => { const handler = (event: Event): void => { diff --git a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx index 7709c4f9545..6a29017d654 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx @@ -10,6 +10,7 @@ import { type FileContent } from './editor-panel-content-types' import { + OWNER_NOT_READY_RETRY_DELAY_MS, OWNER_NOT_READY_RETRY_LIMIT, shouldRetryFileLoadError, useEditorPanelFileLoadRetry @@ -46,19 +47,21 @@ function Harness({ file, fileContents, attemptsRef, + isVisible = true, loadFileContent, setFileContents }: { file: OpenFile fileContents: Record attemptsRef: { current: Record } + isVisible?: boolean loadFileContent: (filePath: string, id: string) => Promise setFileContents: ( updater: (prev: Record) => Record ) => void }): null { useEditorPanelFileLoadRetry({ - activeFile: file, + activeFile: isVisible ? file : null, fileContents, fileLoadRetryAttemptsRef: attemptsRef, loadFileContent: loadFileContent as never, @@ -100,6 +103,67 @@ describe('useEditorPanelFileLoadRetry — owner-not-ready bounding (#6648)', () expect(shouldRetryFileLoadError('Access denied: outside allowed directories')).toBe(false) }) + it('does not spend retry budget when hiding cancels a pending retry', () => { + setTimeoutSpy.mockRestore() + setTimeoutSpy = vi.spyOn(window, 'setTimeout') + const file = makeFile() + const attemptsRef = { current: {} as Record } + const fileContents: Record = { + [file.id]: { content: '', isBinary: false, loadError: WORKTREE_OWNER_NOT_READY_ERROR } + } + const loadFileContent = vi.fn(async () => undefined) + const setFileContents = vi.fn((updater) => updater(fileContents)) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + + act(() => { + root?.render( + + ) + }) + expect(loadFileContent).not.toHaveBeenCalled() + expect(attemptsRef.current[file.id]).toBeUndefined() + + act(() => { + root?.render( + + ) + }) + act(() => vi.advanceTimersByTime(OWNER_NOT_READY_RETRY_DELAY_MS)) + expect(loadFileContent).not.toHaveBeenCalled() + expect(attemptsRef.current[file.id]).toBeUndefined() + + act(() => { + root?.render( + + ) + }) + act(() => vi.advanceTimersByTime(OWNER_NOT_READY_RETRY_DELAY_MS)) + expect(loadFileContent).toHaveBeenCalledOnce() + expect(attemptsRef.current[file.id]).toBe(1) + }) + it('stops after the budget and shows a truthful terminal message, then Retry re-arms', () => { const file = makeFile() const attemptsRef = { current: {} as Record } diff --git a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts index d62bc33aa15..9fb96ad80aa 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts +++ b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts @@ -98,7 +98,6 @@ export function useEditorPanelFileLoadRetry({ const delayMs = ownerNotReady ? OWNER_NOT_READY_RETRY_DELAY_MS : (FILE_LOAD_RETRY_DELAYS_MS[retryCount] ?? FILE_LOAD_RETRY_DELAYS_MS[0]) - fileLoadRetryAttemptsRef.current[activeFileLoadRetryId] = retryCount + 1 const timeoutId = window.setTimeout(() => { const currentFile = openFilesRef.current.find((file) => file.id === activeFileLoadRetryId) if ( @@ -107,6 +106,7 @@ export function useEditorPanelFileLoadRetry({ ) { return } + fileLoadRetryAttemptsRef.current[activeFileLoadRetryId] = retryCount + 1 setFileContents((prev) => { if (prev[currentFile.id]?.loadError !== activeFileLoadError) { return prev diff --git a/src/renderer/src/components/editor/useEditorPanelSave.ts b/src/renderer/src/components/editor/useEditorPanelSave.ts new file mode 100644 index 00000000000..968edcf8ab4 --- /dev/null +++ b/src/renderer/src/components/editor/useEditorPanelSave.ts @@ -0,0 +1,39 @@ +import { useCallback } from 'react' +import type { OpenFile } from '@/store/slices/editor' +import { attemptEditorFileSave } from './editor-file-save-attempt' +import { getEditorSaveTargetFile } from './editor-save-target' + +type UseEditorPanelSaveParams = { + activeFile: OpenFile | null + openFiles: OpenFile[] + requestRenameForFile: (fileId: string) => void +} + +export function useEditorPanelSave({ + activeFile, + openFiles, + requestRenameForFile +}: UseEditorPanelSaveParams) { + const handleSaveForFile = useCallback( + async (file: OpenFile | null, content: string): Promise => { + if (!file) { + return false + } + const saveTargetFile = getEditorSaveTargetFile(file, openFiles) + if (!saveTargetFile) { + return false + } + if (saveTargetFile.isUntitled) { + requestRenameForFile(saveTargetFile.id) + return false + } + return attemptEditorFileSave({ fileId: saveTargetFile.id, fallbackContent: content }) + }, + [openFiles, requestRenameForFile] + ) + const handleSave = useCallback( + (content: string): Promise => handleSaveForFile(activeFile, content), + [activeFile, handleSaveForFile] + ) + return { handleSave, handleSaveForFile } +} diff --git a/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx new file mode 100644 index 00000000000..bfa26936c15 --- /dev/null +++ b/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx @@ -0,0 +1,443 @@ +// @vitest-environment happy-dom + +import { act, useLayoutEffect, useMemo } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import type { GitStatusEntry } from '../../../../shared/types' +import type { DiffContent, FileContent } from './editor-panel-content-types' + +const mocks = vi.hoisted(() => ({ + getRuntimeGitDiff: vi.fn(), + getState: vi.fn(), + readRuntimeFileContent: vi.fn() +})) + +vi.mock('@/runtime/runtime-file-client', () => ({ + getRuntimeFileReadScope: vi.fn(() => null), + readRuntimeFileContent: mocks.readRuntimeFileContent, + subscribeRuntimeFileChanges: vi.fn() +})) + +vi.mock('@/runtime/runtime-git-client', () => ({ + getRuntimeGitBranchDiff: vi.fn(), + getRuntimeGitCommitDiff: vi.fn(), + getRuntimeGitDiff: mocks.getRuntimeGitDiff, + getRuntimeGitScope: vi.fn(() => null) +})) + +vi.mock('@/lib/connection-context', () => ({ + getConnectionId: vi.fn(), + getConnectionIdForFile: vi.fn(), + isWorktreeConnectionResolved: vi.fn(() => true) +})) + +vi.mock('@/lib/runtime-workspace-file-route', () => ({ + findWorkspaceFileRoute: vi.fn(() => null) +})) + +vi.mock('@/store', () => ({ useAppStore: { getState: mocks.getState } })) + +import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT } from './editor-autosave' +import { useEditorPanelContentState } from './useEditorPanelContentState' + +type Deferred = { + promise: Promise + resolve: (value: T) => void +} + +type ProbeSnapshot = { + diffContents: Record + fileContents: Record +} + +type ProbeProps = { + activeFile: OpenFile + editorViewMode?: Record + gitStatusEntries?: GitStatusEntry[] + isChangesMode?: boolean + isVisible?: boolean + name?: string + openFiles?: OpenFile[] +} + +const snapshots = new Map() +const EMPTY_EDITOR_VIEW_MODE: Record = {} + +function createDeferred(): Deferred { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + +function makeFile(id: string, overrides: Partial = {}): OpenFile { + return { + id, + filePath: `/repo/${id}.ts`, + relativePath: `${id}.ts`, + worktreeId: 'wt-1', + language: 'typescript', + isDirty: false, + mode: 'edit', + ...overrides + } +} + +function Probe({ + activeFile, + editorViewMode = EMPTY_EDITOR_VIEW_MODE, + gitStatusEntries, + isChangesMode = false, + isVisible = true, + name = 'main', + openFiles +}: ProbeProps): null { + const panelOpenFiles = useMemo(() => openFiles ?? [activeFile], [activeFile, openFiles]) + const state = useEditorPanelContentState({ + activeFile, + editorViewMode, + gitStatusEntries, + isChangesMode, + isVisible, + openFiles: panelOpenFiles + }) + snapshots.set(name, { + diffContents: state.diffContents, + fileContents: state.fileContents + }) + return null +} + +function dispatchExternalChange(file: OpenFile): void { + act(() => { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, { + detail: { + worktreeId: file.worktreeId, + worktreePath: '/repo', + relativePath: file.relativePath + } + }) + ) + }) +} + +function ExternalChangeLayoutEmitter({ file }: { file: OpenFile }): null { + useLayoutEffect(() => { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, { + detail: { + worktreeId: file.worktreeId, + worktreePath: '/repo', + relativePath: file.relativePath + } + }) + ) + }, [file]) + return null +} + +function textDiff(modifiedContent: string): DiffContent { + return { + kind: 'text', + originalContent: 'old', + modifiedContent, + originalIsBinary: false, + modifiedIsBinary: false + } +} + +describe('useEditorPanelContentState visibility', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + snapshots.clear() + mocks.getRuntimeGitDiff.mockReset() + mocks.readRuntimeFileContent.mockReset() + mocks.getState.mockReset() + mocks.getState.mockReturnValue({ + settings: null, + openFiles: [], + setLastKnownDiskSignature: vi.fn() + }) + container = document.body.appendChild(document.createElement('div')) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + }) + + it('defers the initial remote read until reveal', async () => { + const file = makeFile('initial') + mocks.readRuntimeFileContent.mockResolvedValue({ content: 'remote', isBinary: false }) + + await act(async () => root.render()) + expect(mocks.readRuntimeFileContent).not.toHaveBeenCalled() + + await act(async () => root.render()) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('remote') + ) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + }) + + it('invalidates a hidden tab and rejects its older in-flight result', async () => { + const file = makeFile('stale') + const staleRead = createDeferred() + const freshRead = createDeferred() + mocks.readRuntimeFileContent + .mockReturnValueOnce(staleRead.promise) + .mockReturnValueOnce(freshRead.promise) + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce()) + await act(async () => root.render()) + dispatchExternalChange(file) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + + await act(async () => { + staleRead.resolve({ content: 'stale', isBinary: false }) + await staleRead.promise + }) + expect(snapshots.get('main')?.fileContents[file.id]).toBeUndefined() + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2)) + await act(async () => { + freshRead.resolve({ content: 'fresh', isBinary: false }) + await freshRead.promise + }) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('fresh') + ) + }) + + it('keeps invalidated bytes on screen until the reveal read lands', async () => { + const file = makeFile('no-flash') + const freshRead = createDeferred() + mocks.readRuntimeFileContent + .mockResolvedValueOnce({ content: 'old', isBinary: false }) + .mockReturnValueOnce(freshRead.promise) + + await act(async () => root.render()) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + ) + + await act(async () => root.render()) + dispatchExternalChange(file) + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2)) + // Why: an unresolved reveal read must not blank the pane to "Loading…". + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + + await act(async () => { + freshRead.resolve({ content: 'fresh', isBinary: false }) + await freshRead.promise + }) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]).toEqual({ + content: 'fresh', + isBinary: false + }) + ) + }) + + it('does not re-read while the reveal read is still in flight', async () => { + const file = makeFile('flip-flop') + const pendingRead = createDeferred() + mocks.readRuntimeFileContent.mockReturnValue(pendingRead.promise) + + await act(async () => root.render()) + await act(async () => root.render()) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + + await act(async () => root.render()) + await act(async () => root.render()) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + + await act(async () => { + pendingRead.resolve({ content: 'remote', isBinary: false }) + await pendingRead.promise + }) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('remote') + ) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + }) + + it('publishes hidden visibility before a watcher event in the same commit', async () => { + const file = makeFile('commit-visibility') + mocks.readRuntimeFileContent.mockResolvedValue({ content: 'old', isBinary: false }) + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce()) + + await act(async () => + root.render( + <> + + + + ) + ) + + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + expect(snapshots.get('main')?.fileContents[file.id]).toEqual({ + content: 'old', + isBinary: false, + isStale: true + }) + }) + + it('shares one post-change read between visible source and preview panels', async () => { + const source = makeFile('source', { + filePath: '/repo/readme.md', + relativePath: 'readme.md', + language: 'markdown' + }) + const preview = makeFile('preview', { + filePath: '/repo/readme.md', + relativePath: 'readme.md', + language: 'markdown', + mode: 'markdown-preview', + markdownPreviewSourceFileId: source.id + }) + mocks.readRuntimeFileContent + .mockResolvedValueOnce({ content: 'old', isBinary: false }) + .mockResolvedValueOnce({ content: 'fresh', isBinary: false }) + + await act(async () => { + root.render( + <> + + + + ) + }) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce()) + + dispatchExternalChange(source) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2)) + await vi.waitFor(() => { + expect(snapshots.get('source')?.fileContents[source.id]?.content).toBe('fresh') + expect(snapshots.get('preview')?.fileContents[preview.id]?.content).toBe('fresh') + }) + }) + + it('invalidates a hidden file nonce without reading until reveal', async () => { + const file = makeFile('file-nonce') + mocks.readRuntimeFileContent + .mockResolvedValueOnce({ content: 'old', isBinary: false }) + .mockResolvedValueOnce({ content: 'fresh', isBinary: false }) + + await act(async () => root.render()) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + ) + + const changed = { ...file, fileContentReloadNonce: 1 } + await act(async () => root.render()) + expect(snapshots.get('main')?.fileContents[file.id]?.isStale).toBe(true) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + + await act(async () => root.render()) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('fresh') + ) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2) + }) + + it('invalidates a hidden diff nonce without reading until reveal', async () => { + const file = makeFile('diff-nonce', { mode: 'diff', diffSource: 'unstaged' }) + const staleDiff = createDeferred() + const freshDiff = createDeferred() + mocks.getRuntimeGitDiff + .mockReturnValueOnce(staleDiff.promise) + .mockReturnValueOnce(freshDiff.promise) + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce()) + + const changed = { ...file, diffContentReloadNonce: 1 } + await act(async () => root.render()) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce() + await act(async () => { + staleDiff.resolve(textDiff('stale diff')) + await staleDiff.promise + }) + expect(snapshots.get('main')?.diffContents[file.id]).toBeUndefined() + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2)) + await act(async () => { + freshDiff.resolve(textDiff('fresh diff')) + await freshDiff.promise + }) + await vi.waitFor(() => + expect(snapshots.get('main')?.diffContents[file.id]?.modifiedContent).toBe('fresh diff') + ) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2) + }) + + it('invalidates a hidden Git-status diff without reading until reveal', async () => { + const file = makeFile('status', { mode: 'diff', diffSource: 'unstaged' }) + const status: GitStatusEntry[] = [ + { path: file.relativePath, status: 'modified', area: 'unstaged' } + ] + mocks.getRuntimeGitDiff + .mockResolvedValueOnce(textDiff('old diff')) + .mockResolvedValueOnce(textDiff('fresh diff')) + + await act(async () => root.render()) + await vi.waitFor(() => expect(snapshots.get('main')?.diffContents[file.id]).toBeDefined()) + await act(async () => + root.render() + ) + expect(snapshots.get('main')?.diffContents[file.id]).toBeUndefined() + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce() + + await act(async () => root.render()) + await vi.waitFor(() => + expect(snapshots.get('main')?.diffContents[file.id]?.modifiedContent).toBe('fresh diff') + ) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2) + }) + + it('invalidates an inactive cached Changes diff after an external edit', async () => { + const file = makeFile('changes-cache') + const changesMode = { [file.id]: 'changes' as const } + mocks.readRuntimeFileContent + .mockResolvedValueOnce({ content: 'old', isBinary: false }) + .mockResolvedValueOnce({ content: 'fresh', isBinary: false }) + mocks.getRuntimeGitDiff + .mockResolvedValueOnce(textDiff('old diff')) + .mockResolvedValueOnce(textDiff('fresh diff')) + + await act(async () => + root.render() + ) + await vi.waitFor(() => expect(snapshots.get('main')?.diffContents[file.id]).toBeDefined()) + await act(async () => root.render()) + + dispatchExternalChange(file) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2)) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce() + expect(snapshots.get('main')?.diffContents[file.id]).toBeUndefined() + + await act(async () => + root.render() + ) + await vi.waitFor(() => + expect(snapshots.get('main')?.diffContents[file.id]?.modifiedContent).toBe('fresh diff') + ) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx index 9fc9e12680e..43e74374d9d 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx @@ -2518,6 +2518,16 @@ describe('FloatingTerminalPanel close behavior', () => { expect(editorPanel.props.markdownAnnotationsEnabled).toBe(false) expect(editorPanel.props.activeFileId).toBe('notes') + expect(editorPanel.props.isVisible).toBe(true) + }) + + it('marks the retained floating editor hidden when the panel is closed', async () => { + setFloatingEditorTabs([makeFile({ id: 'notes' })]) + + const element = await renderPanel(false) + const editorPanel = findByProp(element, 'activeFileId') + + expect(editorPanel.props.isVisible).toBe(false) }) it('keeps the panel open when the explicit close action removes the last tab', async () => { diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index 94c0a23160e..1cede3297a4 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -1947,6 +1947,7 @@ export function FloatingTerminalPanel({ diff --git a/src/renderer/src/components/tab-group/TabGroupPanel.tsx b/src/renderer/src/components/tab-group/TabGroupPanel.tsx index 07d8a86ccaf..06b083e1da0 100644 --- a/src/renderer/src/components/tab-group/TabGroupPanel.tsx +++ b/src/renderer/src/components/tab-group/TabGroupPanel.tsx @@ -25,6 +25,7 @@ const EditorPanel = lazy(() => import('../editor/EditorPanel')) export default function TabGroupPanel({ groupId, worktreeId, + isVisible, isFocused, hasSplitGroups, touchesRightEdge, @@ -40,6 +41,7 @@ export default function TabGroupPanel({ }: { groupId: string worktreeId: string + isVisible: boolean isFocused: boolean hasSplitGroups: boolean touchesRightEdge: boolean @@ -324,7 +326,12 @@ export default function TabGroupPanel({ } > - + )} diff --git a/src/renderer/src/components/tab-group/TabGroupSplitLayout.test.ts b/src/renderer/src/components/tab-group/TabGroupSplitLayout.test.ts index 20613d7a2a1..b0cadf8c4b0 100644 --- a/src/renderer/src/components/tab-group/TabGroupSplitLayout.test.ts +++ b/src/renderer/src/components/tab-group/TabGroupSplitLayout.test.ts @@ -100,6 +100,7 @@ describe('TabGroupSplitLayout', () => { return tabGroupPanelElement.props as { groupId: string worktreeId: string + isVisible: boolean isFocused: boolean hasSplitGroups: boolean reserveClosedExplorerToggleSpace: boolean @@ -112,6 +113,7 @@ describe('TabGroupSplitLayout', () => { expect.objectContaining({ groupId: 'group-1', worktreeId: 'wt-1', + isVisible: false, isFocused: false, hasSplitGroups: false, reserveClosedExplorerToggleSpace: true, @@ -125,6 +127,7 @@ describe('TabGroupSplitLayout', () => { expect.objectContaining({ groupId: 'group-1', worktreeId: 'wt-1', + isVisible: true, isFocused: true, hasSplitGroups: false, reserveClosedExplorerToggleSpace: true, diff --git a/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx b/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx index b467e582218..9a97ae673e4 100644 --- a/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx +++ b/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx @@ -182,6 +182,7 @@ function SplitNode({