diff --git a/src/renderer/src/components/editor/ChangesModeView.tsx b/src/renderer/src/components/editor/ChangesModeView.tsx index 7260475dcf4..0e8122fc0d9 100644 --- a/src/renderer/src/components/editor/ChangesModeView.tsx +++ b/src/renderer/src/components/editor/ChangesModeView.tsx @@ -2,18 +2,10 @@ import React, { lazy } from 'react' import type { OpenFile } from '@/store/slices/editor' import type { GitDiffResult, GitStatusEntry } from '../../../../shared/types' import { ConflictBanner } from './ConflictComponents' +import { getDiffContentSignature } from './diff-content-signature' const DiffViewer = lazy(() => import('./DiffViewer')) -function getContentSignature(content: string): string { - let hash = 2166136261 - for (let i = 0; i < content.length; i += 1) { - hash ^= content.charCodeAt(i) - hash = Math.imul(hash, 16777619) - } - return (hash >>> 0).toString(16) -} - // Why: Changes view mode renders an edit-mode tab as a HEAD-vs-working-tree // diff without creating a separate diff-tab object. The draft is the live // source on the modified side; onContentChange is the same callback as normal @@ -70,7 +62,7 @@ export function ChangesModeView({ // diff if we reuse the same kept model identities. Rotate only the // original-side model identity so Monaco rebuilds the stale HEAD snapshot // without throwing away the modified-side undo history. - const headContentSignature = getContentSignature(dc.originalContent) + const headContentSignature = getDiffContentSignature(dc.originalContent) const originalModelKey = `${diffViewStateKey}:original:${headContentSignature}` return (
diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index acc2fdab59d..bbce4491602 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -50,6 +50,11 @@ import { createCombinedDiffSectionIndexMap, handleCombinedDiffFileTreeNavigation } from './CombinedDiffFileTree' +import { getCombinedDiffFileTreeSectionKey } from './combined-diff-file-tree-model' +import { + ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, + type EditorPathMutationTarget +} from './editor-autosave' import { getCombinedBranchEntries, getCombinedUncommittedEntries } from './combined-diff-entries' import { getDiffSectionEstimatedHeight, isIntrinsicHeightImageDiff } from './diff-section-layout' import type { DiffSection } from './diff-section-types' @@ -62,6 +67,7 @@ import { type CachedCombinedDiffViewState = { entrySignature: string + gitStatusSignature: string sections: DiffSection[] sectionHeights: Record loadedIndices: number[] @@ -77,6 +83,42 @@ type CombinedDiffScrollThumb = { const combinedDiffViewStateCache = new Map() const combinedDiffScrollTopCache = new Map() + +function buildCombinedGitStatusSignature( + sections: readonly { path: string }[], + gitStatusEntries: readonly GitStatusEntry[] +): string { + const sectionPaths = new Set(sections.map((section) => section.path)) + const matching = gitStatusEntries.filter((entry) => sectionPaths.has(entry.path)) + return JSON.stringify( + matching.map((entry) => ({ + path: entry.path, + area: entry.area, + status: entry.status, + added: entry.added ?? null, + removed: entry.removed ?? null + })) + ) +} + +function invalidateCombinedDiffCachesForRelativePath(relativePath: string): void { + for (const [key, cached] of combinedDiffViewStateCache.entries()) { + if (cached.sections.some((section) => section.path === relativePath)) { + combinedDiffViewStateCache.delete(key) + } + } +} + +if (typeof window !== 'undefined') { + window.addEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, (event) => { + const detail = (event as CustomEvent).detail + if (detail?.relativePath) { + // Why: inactive combined-diff tabs are unmounted, so only a module-level + // cache bust can prevent a remount from replaying stale section bodies. + invalidateCombinedDiffCachesForRelativePath(detail.relativePath) + } + }) +} const COMBINED_DIFF_OVERSCAN = 5 const COMBINED_DIFF_SCROLLBAR_THUMB_MIN_HEIGHT = 64 const EMPTY_GIT_STATUS_ENTRIES: GitStatusEntry[] = [] @@ -227,6 +269,7 @@ export default function CombinedDiffViewer({ const sectionsRef = useRef([]) const generationRef = useRef(0) const loadSectionRef = useRef<(index: number) => Promise>(async () => {}) + const retrySectionRef = useRef<(index: number) => void>(() => {}) const updateCombinedDiffScrollbar = useCallback(() => { const container = scrollContainerRef.current if (!container || container.scrollHeight <= container.clientHeight + 1) { @@ -401,6 +444,8 @@ export default function CombinedDiffViewer({ const canRestoreCachedSections = cached && cached.entrySignature === entrySignature && + (cached.gitStatusSignature ?? '') === + buildCombinedGitStatusSignature(cached.sections, gitStatusEntries) && (cached.sections.length > 0 || entries.length === 0) if (canRestoreCachedSections && cached) { const collapsedPreference = combinedDiffCollapsedPreference @@ -448,7 +493,7 @@ export default function CombinedDiffViewer({ loadSchedulerRef.current.reset() generationRef.current += 1 setGeneration((prev) => prev + 1) - }, [entries, entrySignature, file.diffSource, viewStateKey]) + }, [entries, entrySignature, file.diffSource, gitStatusEntries, viewStateKey]) const loadSectionNow = useCallback( async (index: number) => { @@ -618,28 +663,41 @@ export default function CombinedDiffViewer({ } }, [entrySignature, loadSection, sections.length]) + const invalidateCombinedDiffViewStateCache = useCallback((): void => { + combinedDiffViewStateCache.delete(viewStateKey) + }, [viewStateKey]) + const retrySection = useCallback( (index: number) => { + const collapsed = sectionsRef.current[index]?.collapsed ?? false loadedIndicesRef.current.delete(index) loadingIndicesRef.current.delete(index) + invalidateCombinedDiffViewStateCache() + generationRef.current += 1 + setGeneration((prev) => prev + 1) setSections((prev) => prev.map((section, sectionIndex) => sectionIndex === index ? { ...section, - loading: true, + loading: !collapsed, error: undefined, diffResult: null, originalContent: '', - modifiedContent: '' + modifiedContent: '', + contentGeneration: (section.contentGeneration ?? 0) + 1 } : section ) ) - loadSection(index) + if (collapsed) { + return + } + loadSchedulerRef.current.rerequest(index) }, - [loadSection] + [invalidateCombinedDiffViewStateCache] ) + retrySectionRef.current = retrySection const modifiedEditorsRef = useRef>(new Map()) @@ -692,6 +750,15 @@ export default function CombinedDiffViewer({ () => createCombinedDiffSectionIndexMap(sections), [sections] ) + const sectionIndexByKeyRef = useRef(sectionIndexByKey) + sectionIndexByKeyRef.current = sectionIndexByKey + const requestCombinedDiffSectionReload = useCallback((index: number): void => { + const section = sectionsRef.current[index] + if (!section || section.dirty) { + return + } + retrySectionRef.current(index) + }, []) const [activeTreeSectionState, setActiveTreeSectionState] = useState<{ entrySignature: string key: string | null @@ -718,15 +785,87 @@ export default function CombinedDiffViewer({ scrollToIndex: (index) => virtualizer.scrollToIndex(index, { align: 'start' }) }) if (navigatedIndex !== null) { + // Why: tree navigation is also the user's explicit "show me this diff" + // affordance. Re-selecting an already-loaded row must refetch in case + // the file or git index changed while the section stayed mounted. + requestCombinedDiffSectionReload(navigatedIndex) setActiveTreeSectionState({ entrySignature, key: sectionsRef.current[navigatedIndex]?.key ?? null }) } }, - [entrySignature, sectionIndexByKey, toggleSection, treeMode, virtualizer] + [ + entrySignature, + requestCombinedDiffSectionReload, + sectionIndexByKey, + toggleSection, + treeMode, + virtualizer + ] ) + const combinedGitStatusSignature = React.useMemo(() => { + if (treeMode !== 'uncommitted') { + return '' + } + return buildCombinedGitStatusSignature(sections, gitStatusEntries) + }, [gitStatusEntries, sections, treeMode]) + const prevCombinedGitStatusSignatureRef = useRef(null) + + useEffect(() => { + if (treeMode !== 'uncommitted') { + prevCombinedGitStatusSignatureRef.current = null + return + } + if (prevCombinedGitStatusSignatureRef.current === null) { + prevCombinedGitStatusSignatureRef.current = combinedGitStatusSignature + return + } + if (prevCombinedGitStatusSignatureRef.current === combinedGitStatusSignature) { + return + } + prevCombinedGitStatusSignatureRef.current = combinedGitStatusSignature + for (const index of loadedIndicesRef.current) { + requestCombinedDiffSectionReload(index) + } + }, [combinedGitStatusSignature, requestCombinedDiffSectionReload, treeMode]) + + useEffect(() => { + if (treeMode !== 'uncommitted') { + return + } + const handler = (event: Event): void => { + const detail = (event as CustomEvent).detail + if (!detail || detail.worktreeId !== file.worktreeId) { + return + } + const hasRuntimeOwnerFilter = Object.prototype.hasOwnProperty.call( + detail, + 'runtimeEnvironmentId' + ) + const targetRuntimeOwner = detail.runtimeEnvironmentId?.trim() || null + const fileRuntimeOwner = file.runtimeEnvironmentId?.trim() || null + if (hasRuntimeOwnerFilter && targetRuntimeOwner !== fileRuntimeOwner) { + return + } + for (const area of ['unstaged', 'staged', 'untracked'] as const) { + const key = getCombinedDiffFileTreeSectionKey('uncommitted', { + path: detail.relativePath, + status: 'modified', + area + }) + const index = sectionIndexByKeyRef.current.get(key) + if (index !== undefined) { + requestCombinedDiffSectionReload(index) + } + } + } + window.addEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, handler as EventListener) + return () => + window.removeEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, handler as EventListener) + }, [file.runtimeEnvironmentId, file.worktreeId, requestCombinedDiffSectionReload, treeMode]) + const setAllSectionsCollapsed = useCallback((collapsed: boolean) => { combinedDiffCollapsedPreference = collapsed setSections((prev) => prev.map((section) => ({ ...section, collapsed }))) @@ -862,6 +1001,7 @@ export default function CombinedDiffViewer({ combinedDiffScrollTopCache.get(viewStateKey) ?? scrollContainerRef.current?.scrollTop ?? 0 setWithLRU(combinedDiffViewStateCache, viewStateKey, { entrySignature, + gitStatusSignature: combinedGitStatusSignature, sections, sectionHeights, loadedIndices: Array.from(loadedIndicesRef.current).filter( @@ -870,7 +1010,15 @@ export default function CombinedDiffViewer({ scrollTop: preservedScrollTop, sideBySide }) - }, [entries.length, entrySignature, sectionHeights, sections, sideBySide, viewStateKey]) + }, [ + combinedGitStatusSignature, + entries.length, + entrySignature, + sectionHeights, + sections, + sideBySide, + viewStateKey + ]) useLayoutEffect(() => { const container = scrollContainerRef.current diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx index a83af3ecbee..482b73645b5 100644 --- a/src/renderer/src/components/editor/DiffSectionItem.tsx +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -108,8 +108,8 @@ export function DiffSectionItem({ const isEditable = section.area === 'unstaged' const modelPathBase = useMemo( () => - `diff-section:${encodeURIComponent(worktreeId ?? 'review')}:${encodeURIComponent(section.key)}`, - [section.key, worktreeId] + `diff-section:${encodeURIComponent(worktreeId ?? 'review')}:${encodeURIComponent(section.key)}:${section.contentGeneration ?? 0}`, + [section.contentGeneration, section.key, worktreeId] ) const diffEditorFontSize = computeDiffEditorFontSize( settings?.terminalFontSize ?? 13, diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index bad4adb0b51..ceee3f2013d 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -26,6 +26,7 @@ import { extractFrontMatter, prependFrontMatter } from './markdown-frontmatter' import { RichMarkdownErrorBoundary } from './RichMarkdownErrorBoundary' import { useMarkdownDocuments } from './useMarkdownDocuments' import { findGitConflictBlocks } from './monaco-conflict-decorations' +import { getDiffContentSignature } from './diff-content-signature' const MonacoEditor = lazy(() => import('./MonacoEditor')) const DiffViewer = lazy(() => import('./DiffViewer')) @@ -845,10 +846,18 @@ export function EditorContent({
) } + // Why: kept Monaco models ignore refreshed git blobs unless the model identity + // rotates. Key off fetched diff content and explicit reload nonce, not live + // edit-buffer text, so editable unstaged diffs keep their undo stack. + const diffReloadNonce = activeFile.diffContentReloadNonce ?? 0 + const originalModelKey = `${diffViewStateKey}:original:${getDiffContentSignature(dc.originalContent)}` + const modifiedModelKey = `${diffViewStateKey}:modified:${getDiffContentSignature(dc.modifiedContent)}:${diffReloadNonce}` return ( { expect(started).toEqual([4, 4]) }) + it('rerequest clears an in-flight queue slot before reloading', async () => { + const blocker = deferred() + const started: number[] = [] + const scheduler = createCombinedDiffLoadScheduler({ + maxConcurrent: 1, + schedule: (callback) => callback(), + loadSection: async (index) => { + started.push(index) + if (index === 4) { + await blocker.promise + } + } + }) + + scheduler.request(4) + scheduler.request(4) + expect(started).toEqual([4]) + + scheduler.rerequest(4) + blocker.resolve() + await flushMicrotasks() + + expect(started).toEqual([4, 4]) + }) + it('drops stale pending work after reset', async () => { const blocker = deferred() const started: number[] = [] diff --git a/src/renderer/src/components/editor/combined-diff-load-scheduler.ts b/src/renderer/src/components/editor/combined-diff-load-scheduler.ts index 6548d9bc892..61c480dd716 100644 --- a/src/renderer/src/components/editor/combined-diff-load-scheduler.ts +++ b/src/renderer/src/components/editor/combined-diff-load-scheduler.ts @@ -1,5 +1,6 @@ export type CombinedDiffLoadScheduler = { request: (index: number) => void + rerequest: (index: number) => void reset: () => void dispose: () => void } @@ -44,15 +45,30 @@ export function createCombinedDiffLoadScheduler({ } } + const enqueue = (index: number): void => { + if (disposed || queued.has(index)) { + return + } + queued.add(index) + pending.push(index) + const requestVersion = version + schedule(() => drain(requestVersion)) + } + return { request(index) { - if (disposed || queued.has(index)) { + enqueue(index) + }, + rerequest(index) { + if (disposed) { return } - queued.add(index) - pending.push(index) - const requestVersion = version - schedule(() => drain(requestVersion)) + queued.delete(index) + const pendingIndex = pending.indexOf(index) + if (pendingIndex !== -1) { + pending.splice(pendingIndex, 1) + } + enqueue(index) }, reset() { disposed = false diff --git a/src/renderer/src/components/editor/diff-content-signature.ts b/src/renderer/src/components/editor/diff-content-signature.ts new file mode 100644 index 00000000000..927d9711d89 --- /dev/null +++ b/src/renderer/src/components/editor/diff-content-signature.ts @@ -0,0 +1,11 @@ +// Why: Monaco diff tabs keep models alive via keepCurrent*Model. Rotating model +// identities when git-fetched blob content changes forces a fresh paint without +// remounting on every editable keystroke. +export function getDiffContentSignature(content: string): string { + let hash = 2166136261 + for (let i = 0; i < content.length; i += 1) { + hash ^= content.charCodeAt(i) + hash = Math.imul(hash, 16777619) + } + return (hash >>> 0).toString(16) +} diff --git a/src/renderer/src/components/editor/diff-section-types.ts b/src/renderer/src/components/editor/diff-section-types.ts index 50c5bff273a..280d19c15ab 100644 --- a/src/renderer/src/components/editor/diff-section-types.ts +++ b/src/renderer/src/components/editor/diff-section-types.ts @@ -15,4 +15,7 @@ export type DiffSection = { error?: string dirty: boolean diffResult: GitDiffResult | null + // Why: combined sections keep Monaco models by path; bump on reload so + // refetched git content does not replay through keepCurrent* model reuse. + contentGeneration?: number } diff --git a/src/renderer/src/components/editor/editor-autosave.test.ts b/src/renderer/src/components/editor/editor-autosave.test.ts index 883978c7966..20d32f8f6a7 100644 --- a/src/renderer/src/components/editor/editor-autosave.test.ts +++ b/src/renderer/src/components/editor/editor-autosave.test.ts @@ -3,6 +3,7 @@ import type { OpenFile } from '@/store/slices/editor' import { canAutoSaveOpenFile, getOpenFilesForExternalFileChange, + isExternalReloadableEditorTab, normalizeAutoSaveDelayMs, ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, ORCA_EDITOR_QUIESCE_FILE_SAVES_EVENT, @@ -157,6 +158,32 @@ describe('requestEditorFileClose', () => { }) }) +describe('isExternalReloadableEditorTab', () => { + it('includes edit, preview, and single-file staged/unstaged diff tabs', () => { + expect(isExternalReloadableEditorTab(makeOpenFile())).toBe(true) + expect( + isExternalReloadableEditorTab( + makeOpenFile({ mode: 'markdown-preview', language: 'markdown' }) + ) + ).toBe(true) + expect( + isExternalReloadableEditorTab( + makeOpenFile({ mode: 'diff', diffSource: 'unstaged', id: 'diff-unstaged' }) + ) + ).toBe(true) + expect( + isExternalReloadableEditorTab( + makeOpenFile({ mode: 'diff', diffSource: 'staged', id: 'diff-staged' }) + ) + ).toBe(true) + expect( + isExternalReloadableEditorTab( + makeOpenFile({ mode: 'diff', diffSource: 'combined-uncommitted', id: 'combined' }) + ) + ).toBe(false) + }) +}) + describe('getOpenFilesForExternalFileChange', () => { it('matches edit tabs and unstaged diff tabs for the same worktree file', () => { const matchingEdit = makeOpenFile() @@ -191,7 +218,12 @@ describe('getOpenFilesForExternalFileChange', () => { relativePath: 'file.ts' } ).map((file) => file.id) - ).toEqual(['/repo/file.ts', 'markdown-preview::/repo/file.ts', 'wt-1::diff::unstaged::file.ts']) + ).toEqual([ + '/repo/file.ts', + 'markdown-preview::/repo/file.ts', + 'wt-1::diff::unstaged::file.ts', + 'wt-1::diff::staged::file.ts' + ]) }) it('filters same-path matches by runtime owner when the watcher supplies one', () => { diff --git a/src/renderer/src/components/editor/editor-autosave.ts b/src/renderer/src/components/editor/editor-autosave.ts index a1261ab45bb..57b9f9a6fea 100644 --- a/src/renderer/src/components/editor/editor-autosave.ts +++ b/src/renderer/src/components/editor/editor-autosave.ts @@ -49,6 +49,14 @@ export type EditorRequestFileCloseDetail = { fileId: string } +export function isExternalReloadableEditorTab(file: OpenFile): boolean { + return ( + file.mode === 'edit' || + file.mode === 'markdown-preview' || + (file.mode === 'diff' && (file.diffSource === 'unstaged' || file.diffSource === 'staged')) + ) +} + 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 @@ -91,7 +99,10 @@ export function getOpenFilesForExternalFileChange( return file.filePath === absolutePath } if (file.mode === 'diff') { - return file.diffSource === 'unstaged' && file.relativePath === target.relativePath + return ( + (file.diffSource === 'unstaged' || file.diffSource === 'staged') && + file.relativePath === target.relativePath + ) } return false }) diff --git a/src/renderer/src/components/editor/editor-panel-diff-reload.test.ts b/src/renderer/src/components/editor/editor-panel-diff-reload.test.ts new file mode 100644 index 00000000000..3c1b8b1cfdc --- /dev/null +++ b/src/renderer/src/components/editor/editor-panel-diff-reload.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { + isReloadableSingleFileDiffTab, + shouldReloadDiffOnGitStatusChange +} from './editor-panel-diff-reload' + +function makeDiffFile(overrides: Partial = {}): OpenFile { + return { + id: 'wt-1::diff::unstaged::file.ts', + filePath: '/repo/file.ts', + relativePath: 'file.ts', + worktreeId: 'wt-1', + language: 'typescript', + isDirty: false, + mode: 'diff', + diffSource: 'unstaged', + ...overrides + } +} + +describe('editor-panel-diff-reload helpers', () => { + it('treats single-file diff tabs as reloadable', () => { + expect(isReloadableSingleFileDiffTab(makeDiffFile())).toBe(true) + expect(isReloadableSingleFileDiffTab(makeDiffFile({ diffSource: 'staged' }))).toBe(true) + expect(isReloadableSingleFileDiffTab(makeDiffFile({ diffSource: 'branch' }))).toBe(true) + expect( + isReloadableSingleFileDiffTab(makeDiffFile({ diffSource: 'combined-uncommitted' })) + ).toBe(false) + }) + + it('reloads unstaged and staged diff tabs when git status changes', () => { + expect(shouldReloadDiffOnGitStatusChange(makeDiffFile())).toBe(true) + expect(shouldReloadDiffOnGitStatusChange(makeDiffFile({ diffSource: 'staged' }))).toBe(true) + expect(shouldReloadDiffOnGitStatusChange(makeDiffFile({ diffSource: 'branch' }))).toBe(false) + expect(shouldReloadDiffOnGitStatusChange(makeDiffFile({ mode: 'edit' }))).toBe(false) + }) +}) diff --git a/src/renderer/src/components/editor/editor-panel-diff-reload.ts b/src/renderer/src/components/editor/editor-panel-diff-reload.ts new file mode 100644 index 00000000000..0f7ec409e6d --- /dev/null +++ b/src/renderer/src/components/editor/editor-panel-diff-reload.ts @@ -0,0 +1,15 @@ +import type { OpenFile } from '@/store/slices/editor' + +export function isReloadableSingleFileDiffTab(file: OpenFile): boolean { + return ( + file.mode === 'diff' && + file.diffSource !== undefined && + file.diffSource !== 'combined-uncommitted' && + file.diffSource !== 'combined-branch' && + file.diffSource !== 'combined-commit' + ) +} + +export function shouldReloadDiffOnGitStatusChange(file: OpenFile): boolean { + return file.mode === 'diff' && (file.diffSource === 'unstaged' || file.diffSource === 'staged') +} diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index 3da0717b3da..b611e0a98c0 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, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { OpenFile } from '@/store/slices/editor' import { getConnectionId } from '@/lib/connection-context' import { joinPath } from '@/lib/path' @@ -16,6 +16,10 @@ import { } from '@/runtime/runtime-git-client' import type { DiffContent, FileContent } from './editor-panel-content-types' import { canUseChangesModeForFile } from './editor-panel-file-mode' +import { + isReloadableSingleFileDiffTab, + shouldReloadDiffOnGitStatusChange +} from './editor-panel-diff-reload' import { useEditorPanelExternalContentEvents, usePruneClosedEditorContent @@ -71,6 +75,8 @@ export function useEditorPanelContentState({ }: UseEditorPanelContentStateParams): UseEditorPanelContentStateResult { const [fileContents, setFileContents] = useState>({}) const [diffContents, setDiffContents] = useState>({}) + const diffContentsRef = useRef(diffContents) + diffContentsRef.current = diffContents const fileLoadRetryAttemptsRef = useRef>({}) const openFilesRef = useRef(openFiles) openFilesRef.current = openFiles @@ -139,107 +145,113 @@ export function useEditorPanelContentState({ [] ) - const loadDiffContent = useCallback(async (file: OpenFile | null): Promise => { - if (!file || (file.mode === 'edit' && !canUseChangesModeForFile(file))) { - return - } - try { - const worktreePath = file.filePath.slice( - 0, - file.filePath.length - file.relativePath.length - 1 - ) - const branchCompare = - file.branchCompare?.baseOid && file.branchCompare.headOid && file.branchCompare.mergeBase - ? file.branchCompare - : null - const commitCompare = file.commitCompare?.commitOid ? file.commitCompare : null - const connectionId = getConnectionId(file.worktreeId) ?? undefined - const activeSettings = useAppStore.getState().settings - const fileSettings = settingsForRuntimeOwner(activeSettings, file.runtimeEnvironmentId) - const gitScope = getRuntimeGitScope(fileSettings, connectionId) - const effectiveDiffSource: typeof file.diffSource = - file.mode === 'edit' ? 'unstaged' : file.diffSource - const compareAgainstHead = file.mode === 'edit' - const key = inFlightDiffKey( - { ...file, diffSource: effectiveDiffSource }, - gitScope ?? undefined, - compareAgainstHead - ) - let pending = inFlightDiffReads.get(key) - if (!pending) { - pending = ( - effectiveDiffSource === 'commit' - ? commitCompare - ? getRuntimeGitCommitDiff( - { - settings: fileSettings, - worktreeId: file.worktreeId, - worktreePath, - connectionId - }, - { - commitOid: commitCompare.commitOid, - parentOid: commitCompare.parentOid, - filePath: file.relativePath, - oldPath: file.branchOldPath - } - ) - : Promise.reject(new Error('Missing commit comparison for diff tab.')) - : effectiveDiffSource === 'branch' && branchCompare - ? getRuntimeGitBranchDiff( - { - settings: fileSettings, - worktreeId: file.worktreeId, - worktreePath, - connectionId - }, - { - compare: { - baseRef: branchCompare.baseRef, - baseOid: branchCompare.baseOid!, - headOid: branchCompare.headOid!, - mergeBase: branchCompare.mergeBase! - }, - filePath: file.relativePath, - oldPath: file.branchOldPath - } - ) - : getRuntimeGitDiff( - { - settings: fileSettings, - worktreeId: file.worktreeId, - worktreePath, - connectionId - }, - { - filePath: file.relativePath, - staged: effectiveDiffSource === 'staged', - compareAgainstHead - } - ) - ) as Promise - inFlightDiffReads.set(key, pending) - queueMicrotask(() => { - if (inFlightDiffReads.get(key) === pending) { - inFlightDiffReads.delete(key) - } - }) + const loadDiffContent = useCallback( + async (file: OpenFile | null, options?: { force?: boolean }): Promise => { + if (!file || (file.mode === 'edit' && !canUseChangesModeForFile(file))) { + return } - const result = await pending - setDiffContents((prev) => ({ ...prev, [file.id]: result })) - } catch (err) { - setDiffContents((prev) => ({ - ...prev, - [file.id]: { - kind: 'text', - originalContent: '', - modifiedContent: `Error loading diff: ${err}`, - originalIsBinary: false, - modifiedIsBinary: false + try { + const worktreePath = file.filePath.slice( + 0, + file.filePath.length - file.relativePath.length - 1 + ) + const branchCompare = + file.branchCompare?.baseOid && file.branchCompare.headOid && file.branchCompare.mergeBase + ? file.branchCompare + : null + const commitCompare = file.commitCompare?.commitOid ? file.commitCompare : null + const connectionId = getConnectionId(file.worktreeId) ?? undefined + const activeSettings = useAppStore.getState().settings + const fileSettings = settingsForRuntimeOwner(activeSettings, file.runtimeEnvironmentId) + const gitScope = getRuntimeGitScope(fileSettings, connectionId) + const effectiveDiffSource: typeof file.diffSource = + file.mode === 'edit' ? 'unstaged' : file.diffSource + const compareAgainstHead = file.mode === 'edit' + const key = inFlightDiffKey( + { ...file, diffSource: effectiveDiffSource }, + gitScope ?? undefined, + compareAgainstHead + ) + if (options?.force) { + inFlightDiffReads.delete(key) } - })) - } - }, []) + let pending = inFlightDiffReads.get(key) + if (!pending) { + pending = ( + effectiveDiffSource === 'commit' + ? commitCompare + ? getRuntimeGitCommitDiff( + { + settings: fileSettings, + worktreeId: file.worktreeId, + worktreePath, + connectionId + }, + { + commitOid: commitCompare.commitOid, + parentOid: commitCompare.parentOid, + filePath: file.relativePath, + oldPath: file.branchOldPath + } + ) + : Promise.reject(new Error('Missing commit comparison for diff tab.')) + : effectiveDiffSource === 'branch' && branchCompare + ? getRuntimeGitBranchDiff( + { + settings: fileSettings, + worktreeId: file.worktreeId, + worktreePath, + connectionId + }, + { + compare: { + baseRef: branchCompare.baseRef, + baseOid: branchCompare.baseOid!, + headOid: branchCompare.headOid!, + mergeBase: branchCompare.mergeBase! + }, + filePath: file.relativePath, + oldPath: file.branchOldPath + } + ) + : getRuntimeGitDiff( + { + settings: fileSettings, + worktreeId: file.worktreeId, + worktreePath, + connectionId + }, + { + filePath: file.relativePath, + staged: effectiveDiffSource === 'staged', + compareAgainstHead + } + ) + ) as Promise + inFlightDiffReads.set(key, pending) + queueMicrotask(() => { + if (inFlightDiffReads.get(key) === pending) { + inFlightDiffReads.delete(key) + } + }) + } + const result = await pending + setDiffContents((prev) => ({ ...prev, [file.id]: result })) + } catch (err) { + setDiffContents((prev) => ({ + ...prev, + [file.id]: { + kind: 'text', + originalContent: '', + modifiedContent: `Error loading diff: ${err}`, + originalIsBinary: false, + modifiedIsBinary: false + } + })) + } + }, + [] + ) const reloadFileContent = useCallback( (file: OpenFile): void => { @@ -298,14 +310,7 @@ export function useEditorPanelContentState({ if (isChangesMode && !diffContents[fileToLoad.id]) { void loadDiffContent(fileToLoad) } - } else if ( - fileToLoad.mode === 'diff' && - fileToLoad.diffSource !== undefined && - fileToLoad.diffSource !== 'combined-uncommitted' && - fileToLoad.diffSource !== 'combined-branch' && - fileToLoad.diffSource !== 'combined-commit' && - !diffContents[fileToLoad.id] - ) { + } else if (isReloadableSingleFileDiffTab(fileToLoad) && !diffContents[fileToLoad.id]) { void loadDiffContent(fileToLoad) } // oxlint-disable-next-line react-hooks/exhaustive-deps @@ -331,22 +336,57 @@ export function useEditorPanelContentState({ const changesStatusEntries = activeFile?.worktreeId ? gitStatusByWorktree[activeFile.worktreeId] : undefined + const activeFileGitStatusSignature = useMemo(() => { + if (!activeFile?.relativePath || !changesStatusEntries) { + return '' + } + const matching = changesStatusEntries.filter((entry) => entry.path === activeFile.relativePath) + return JSON.stringify( + matching.map((entry) => ({ + area: entry.area, + status: entry.status, + conflictStatus: entry.conflictStatus + })) + ) + }, [activeFile?.relativePath, changesStatusEntries]) useEffect(() => { - if (!isChangesMode || !activeFile?.id) { + if (!activeFile?.id) { return } const current = openFilesRef.current.find((f) => f.id === activeFile.id) - if (current) { - void loadDiffContent(current) + if (!current) { + return } - }, [ - changesStatusEntries, - isChangesMode, - activeFile?.id, - activeFile?.worktreeId, - activeFile?.relativePath, - loadDiffContent - ]) + if (!(isChangesMode || shouldReloadDiffOnGitStatusChange(current))) { + 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]) { + return + } + void loadDiffContent(current, { force: true }) + }, [activeFileGitStatusSignature, isChangesMode, activeFile?.id, loadDiffContent]) + + useEffect(() => { + const nonce = activeFile?.diffContentReloadNonce + if (!activeFile?.id || nonce === undefined || nonce === 0) { + return + } + const current = openFilesRef.current.find((f) => f.id === activeFile.id) + if (!current || !isReloadableSingleFileDiffTab(current)) { + return + } + setDiffContents((prev) => { + if (!prev[current.id]) { + return prev + } + const next = { ...prev } + delete next[current.id] + return next + }) + void loadDiffContent(current, { force: true }) + }, [activeFile?.diffContentReloadNonce, activeFile?.id, loadDiffContent]) useEditorPanelExternalContentEvents({ loadDiffContent, diff --git a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts index bcda787907a..7a93c6c857a 100644 --- a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts +++ b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts @@ -9,11 +9,12 @@ import { type EditorPathMutationTarget } from './editor-autosave' import type { DiffContent, FileContent } from './editor-panel-content-types' +import { isReloadableSingleFileDiffTab } from './editor-panel-diff-reload' type EditorViewModeByFile = ReturnType['editorViewMode'] type UseEditorPanelExternalContentEventsParams = { - loadDiffContent: (file: OpenFile | null) => Promise + loadDiffContent: (file: OpenFile | null, options?: { force?: boolean }) => Promise loadFileContent: (filePath: string, id: string, worktreeId?: string) => Promise openFilesRef: MutableRefObject editorViewModeRef: MutableRefObject @@ -39,15 +40,10 @@ export function useEditorPanelExternalContentEvents({ if (file.mode === 'edit' || file.mode === 'markdown-preview') { void loadFileContent(file.filePath, file.id, file.worktreeId) if (editorViewModeRef.current[file.id] === 'changes') { - void loadDiffContent(file) + void loadDiffContent(file, { force: true }) } - } else if ( - file.mode === 'diff' && - file.diffSource !== 'combined-uncommitted' && - file.diffSource !== 'combined-branch' && - file.diffSource !== 'combined-commit' - ) { - void loadDiffContent(file) + } else if (isReloadableSingleFileDiffTab(file)) { + void loadDiffContent(file, { force: true }) } } } diff --git a/src/renderer/src/hooks/useEditorExternalWatch.test.ts b/src/renderer/src/hooks/useEditorExternalWatch.test.ts index c483d0c3821..0494728a60d 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch.test.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi, afterEach } from 'vitest' +import type * as EditorAutosaveModule from '@/components/editor/editor-autosave' import type { FsChangedPayload } from '../../../shared/types' vi.mock('@/store', () => ({ @@ -9,10 +10,14 @@ vi.mock('@/store', () => ({ // Why: editor-autosave calls window.dispatchEvent at module scope paths; the // vitest 'node' environment has no window. Stub the two exports we use so the // handler can run headlessly. -vi.mock('@/components/editor/editor-autosave', () => ({ - notifyEditorExternalFileChange: vi.fn(), - getOpenFilesForExternalFileChange: vi.fn(() => []) -})) +vi.mock('@/components/editor/editor-autosave', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + notifyEditorExternalFileChange: vi.fn(), + getOpenFilesForExternalFileChange: vi.fn(() => []) + } +}) import { createExternalWatchEventHandler, @@ -118,6 +123,12 @@ describe('getOverflowExternalReloadTargets', () => { worktreePath: '/repo', relativePath: 'notes.md', runtimeEnvironmentId: null + }, + { + worktreeId: 'wt-1', + worktreePath: '/repo', + relativePath: 'staged.ts', + runtimeEnvironmentId: null } ]) expect(setExternalMutation).toHaveBeenCalledWith('file-1', null) diff --git a/src/renderer/src/hooks/useEditorExternalWatch.ts b/src/renderer/src/hooks/useEditorExternalWatch.ts index 9e29725d07b..c32d4678c18 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch.ts @@ -9,6 +9,7 @@ import { getExternalFileChangeRelativePath } from '@/components/right-sidebar/us import { normalizeRuntimePathForComparison } from '../../../shared/cross-platform-path' import { getOpenFilesForExternalFileChange, + isExternalReloadableEditorTab, notifyEditorExternalFileChange } from '@/components/editor/editor-autosave' import { @@ -651,7 +652,7 @@ export function getOverflowExternalReloadTargets( if ( file.worktreeId !== target.worktreeId || openFileRuntimeOwner(file) !== (target.runtimeEnvironmentId ?? null) || - (file.mode !== 'edit' && file.mode !== 'markdown-preview') || + !isExternalReloadableEditorTab(file) || file.isDirty ) { continue diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index d39c5d714d2..0ada3d2b836 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -342,6 +342,19 @@ describe('createEditorSlice openDiff', () => { expect(store.getState().activeFileId).toBe('wt-1::diff::staged::file.ts') }) + it('bumps diffContentReloadNonce when re-opening an existing diff tab', () => { + const store = createEditorStore() + + store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', false) + expect(store.getState().openFiles[0]?.diffContentReloadNonce).toBeUndefined() + + store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', false) + expect(store.getState().openFiles[0]?.diffContentReloadNonce).toBe(1) + + store.getState().openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', false) + expect(store.getState().openFiles[0]?.diffContentReloadNonce).toBe(2) + }) + it('opens the visible diff tab in the requested split group', () => { const store = createEditorTabsStore() const sourceTab = store.getState().createUnifiedTab('wt-1', 'terminal', { id: 'terminal-1' }) diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index bbac165595d..fe823970eb8 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -182,6 +182,10 @@ export type OpenFile = { // a strikethrough label plus a "deleted"/"renamed" suffix. Cleared if the // file reappears on disk at its original path. externalMutation?: 'deleted' | 'renamed' + /** Why: diff bodies are cached in EditorPanel. Re-selecting an existing diff + * tab from the tree bumps this so the panel refetches instead of reusing a + * stale snapshot. */ + diffContentReloadNonce?: number mode: 'edit' | 'diff' | 'conflict-review' | 'markdown-preview' } @@ -794,6 +798,13 @@ function buildDiffEditorFileId( : legacyId } +function withDiffContentReloadRequest(file: OpenFile): OpenFile { + return { + ...file, + diffContentReloadNonce: (file.diffContentReloadNonce ?? 0) + 1 + } +} + function isEditorFileIdOccupiedByOtherOwner( file: Pick< OpenFile, @@ -2291,28 +2302,18 @@ export const createEditorSlice: StateCreator = (s const existing = s.openFiles.find((f) => f.id === id) if (existing) { const updatedPreview = isPreview ? existing.isPreview : false - const needsUpdate = - existing.mode !== 'diff' || - existing.diffSource !== diffSource || - existing.isPreview !== updatedPreview || - existing.runtimeEnvironmentId !== runtimeEnvironmentId + const reopenedDiff = withDiffContentReloadRequest({ + ...existing, + mode: 'diff' as const, + diffSource, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined, + isPreview: updatedPreview, + runtimeEnvironmentId + }) return { - openFiles: needsUpdate - ? s.openFiles.map((f) => - f.id === id - ? { - ...f, - mode: 'diff' as const, - diffSource, - conflict: undefined, - skippedConflicts: undefined, - conflictReview: undefined, - isPreview: updatedPreview, - runtimeEnvironmentId - } - : f - ) - : s.openFiles, + openFiles: s.openFiles.map((f) => (f.id === id ? reopenedDiff : f)), activeFileId: id, activeTabType: 'editor', activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id }, @@ -2383,22 +2384,19 @@ export const createEditorSlice: StateCreator = (s const existing = s.openFiles.find((f) => f.id === id) if (existing) { const updatedPreview = isPreview ? existing.isPreview : false + const reopenedDiff = withDiffContentReloadRequest({ + ...existing, + mode: 'diff' as const, + diffSource: 'branch' as const, + branchCompare, + branchOldPath: entry.oldPath, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined, + isPreview: updatedPreview + }) return { - openFiles: s.openFiles.map((f) => - f.id === id - ? { - ...f, - mode: 'diff' as const, - diffSource: 'branch' as const, - branchCompare, - branchOldPath: entry.oldPath, - conflict: undefined, - skippedConflicts: undefined, - conflictReview: undefined, - isPreview: updatedPreview - } - : f - ), + openFiles: s.openFiles.map((f) => (f.id === id ? reopenedDiff : f)), activeFileId: id, activeTabType: 'editor', activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id }, @@ -2470,22 +2468,19 @@ export const createEditorSlice: StateCreator = (s const existing = s.openFiles.find((f) => f.id === id) if (existing) { const updatedPreview = isPreview ? existing.isPreview : false + const reopenedDiff = withDiffContentReloadRequest({ + ...existing, + mode: 'diff' as const, + diffSource: 'commit' as const, + commitCompare, + branchOldPath: entry.oldPath, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined, + isPreview: updatedPreview + }) return { - openFiles: s.openFiles.map((f) => - f.id === id - ? { - ...f, - mode: 'diff' as const, - diffSource: 'commit' as const, - commitCompare, - branchOldPath: entry.oldPath, - conflict: undefined, - skippedConflicts: undefined, - conflictReview: undefined, - isPreview: updatedPreview - } - : f - ), + openFiles: s.openFiles.map((f) => (f.id === id ? reopenedDiff : f)), activeFileId: id, activeTabType: 'editor', activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id },