diff --git a/src/main/runtime/headless-tab-group-split-layout.ts b/src/main/runtime/headless-tab-group-split-layout.ts index 91167f0bc..da7351d49 100644 --- a/src/main/runtime/headless-tab-group-split-layout.ts +++ b/src/main/runtime/headless-tab-group-split-layout.ts @@ -193,8 +193,7 @@ export function buildHeadlessTabGroupSplit(args: { return { ...group, tabOrder: sourceOrder, - activeTabId: - group.activeTabId === args.tabId ? (sourceOrder[0] ?? null) : group.activeTabId + activeTabId: group.activeTabId === args.tabId ? (sourceOrder[0] ?? null) : group.activeTabId } } return group diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index 8cb8d0455..1a09dc27e 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -55,7 +55,11 @@ import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, type EditorPathMutationTarget } from './editor-autosave' -import { getCombinedBranchEntries, getCombinedUncommittedEntries } from './combined-diff-entries' +import { + getCombinedBranchEntries, + getCombinedUncommittedEntries, + shouldAutoReloadCombinedDiffFromGitStatus +} from './combined-diff-entries' import { getCombinedDiffCommitMessageBody } from './combined-diff-commit-message' import { getDiffSectionEstimatedHeight, isIntrinsicHeightImageDiff } from './diff-section-layout' import { getLargeDiffRenderLimit } from './large-diff-render-limit' @@ -387,6 +391,11 @@ export default function CombinedDiffViewer({ ) const entries = isBranchMode ? branchEntries : isCommitMode ? commitEntries : uncommittedEntries const treeMode = isBranchMode ? 'branch' : isCommitMode ? 'commit' : 'uncommitted' + const hasUncommittedEntriesSnapshot = file.uncommittedEntriesSnapshot !== undefined + const shouldAutoReloadFromGitStatus = shouldAutoReloadCombinedDiffFromGitStatus({ + mode: treeMode, + hasUncommittedEntriesSnapshot + }) const entrySignature = React.useMemo( () => JSON.stringify({ @@ -820,15 +829,15 @@ export default function CombinedDiffViewer({ ) const combinedGitStatusSignature = React.useMemo(() => { - if (treeMode !== 'uncommitted') { + if (!shouldAutoReloadFromGitStatus) { return '' } return buildCombinedGitStatusSignature(sections, gitStatusEntries) - }, [gitStatusEntries, sections, treeMode]) + }, [gitStatusEntries, sections, shouldAutoReloadFromGitStatus]) const prevCombinedGitStatusSignatureRef = useRef(null) useEffect(() => { - if (treeMode !== 'uncommitted') { + if (!shouldAutoReloadFromGitStatus) { prevCombinedGitStatusSignatureRef.current = null return } @@ -843,7 +852,7 @@ export default function CombinedDiffViewer({ for (const index of loadedIndicesRef.current) { requestCombinedDiffSectionReload(index) } - }, [combinedGitStatusSignature, requestCombinedDiffSectionReload, treeMode]) + }, [combinedGitStatusSignature, requestCombinedDiffSectionReload, shouldAutoReloadFromGitStatus]) useEffect(() => { if (treeMode !== 'uncommitted') { diff --git a/src/renderer/src/components/editor/combined-diff-entries.test.ts b/src/renderer/src/components/editor/combined-diff-entries.test.ts index d16fa0392..2aa493556 100644 --- a/src/renderer/src/components/editor/combined-diff-entries.test.ts +++ b/src/renderer/src/components/editor/combined-diff-entries.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { getCombinedBranchEntries, getCombinedUncommittedEntries } from './combined-diff-entries' +import { + getCombinedBranchEntries, + getCombinedUncommittedEntries, + shouldAutoReloadCombinedDiffFromGitStatus +} from './combined-diff-entries' import type { GitBranchChangeEntry, GitStatusEntry } from '../../../../shared/types' describe('getCombinedUncommittedEntries', () => { @@ -58,3 +62,38 @@ describe('getCombinedBranchEntries', () => { expect(getCombinedBranchEntries(undefined, liveEntries)).toEqual(liveEntries) }) }) + +describe('shouldAutoReloadCombinedDiffFromGitStatus', () => { + it('does not auto-reload snapshot-backed uncommitted diffs', () => { + expect( + shouldAutoReloadCombinedDiffFromGitStatus({ + mode: 'uncommitted', + hasUncommittedEntriesSnapshot: true + }) + ).toBe(false) + }) + + it('keeps the legacy live-entry uncommitted path reloadable', () => { + expect( + shouldAutoReloadCombinedDiffFromGitStatus({ + mode: 'uncommitted', + hasUncommittedEntriesSnapshot: false + }) + ).toBe(true) + }) + + it('does not use git status to reload branch or commit combined diffs', () => { + expect( + shouldAutoReloadCombinedDiffFromGitStatus({ + mode: 'branch', + hasUncommittedEntriesSnapshot: false + }) + ).toBe(false) + expect( + shouldAutoReloadCombinedDiffFromGitStatus({ + mode: 'commit', + hasUncommittedEntriesSnapshot: false + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/components/editor/combined-diff-entries.ts b/src/renderer/src/components/editor/combined-diff-entries.ts index 147601382..c2ca3526d 100644 --- a/src/renderer/src/components/editor/combined-diff-entries.ts +++ b/src/renderer/src/components/editor/combined-diff-entries.ts @@ -1,5 +1,6 @@ import type { OpenFile } from '@/store/slices/editor' import type { GitBranchChangeEntry, GitStatusEntry } from '../../../../shared/types' +import type { CombinedDiffFileTreeMode } from './combined-diff-file-tree-model' /** * Fallback filtering for combined-diff tabs that were opened before the @@ -29,3 +30,15 @@ export function getCombinedBranchEntries( // to later Source Control refreshes. return [...(snapshotEntries ?? liveEntries)] } + +export function shouldAutoReloadCombinedDiffFromGitStatus({ + mode, + hasUncommittedEntriesSnapshot +}: { + mode: CombinedDiffFileTreeMode + hasUncommittedEntriesSnapshot: boolean +}): boolean { + // Why: snapshot-backed tabs intentionally preserve the tab-open diff while + // staging/commit status churns; targeted editor-write reloads still refresh. + return mode === 'uncommitted' && !hasUncommittedEntriesSnapshot +} 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 index 3c1b8b1cf..2463ce680 100644 --- a/src/renderer/src/components/editor/editor-panel-diff-reload.test.ts +++ b/src/renderer/src/components/editor/editor-panel-diff-reload.test.ts @@ -4,6 +4,7 @@ import { isReloadableSingleFileDiffTab, shouldReloadDiffOnGitStatusChange } from './editor-panel-diff-reload' +import type { GitStatusEntry } from '../../../../shared/types' function makeDiffFile(overrides: Partial = {}): OpenFile { return { @@ -35,4 +36,34 @@ describe('editor-panel-diff-reload helpers', () => { expect(shouldReloadDiffOnGitStatusChange(makeDiffFile({ diffSource: 'branch' }))).toBe(false) expect(shouldReloadDiffOnGitStatusChange(makeDiffFile({ mode: 'edit' }))).toBe(false) }) + + it('keeps an unstaged diff snapshot when the row moves to staged', () => { + const entries: GitStatusEntry[] = [{ path: 'file.ts', status: 'modified', area: 'staged' }] + + expect(shouldReloadDiffOnGitStatusChange(makeDiffFile(), entries)).toBe(false) + }) + + it('keeps a staged diff snapshot when the row is committed', () => { + expect(shouldReloadDiffOnGitStatusChange(makeDiffFile({ diffSource: 'staged' }), [])).toBe( + false + ) + }) + + it('reloads a diff when its own status row is still present', () => { + expect( + shouldReloadDiffOnGitStatusChange(makeDiffFile(), [ + { path: 'file.ts', status: 'modified', area: 'unstaged' } + ]) + ).toBe(true) + expect( + shouldReloadDiffOnGitStatusChange(makeDiffFile(), [ + { path: 'file.ts', status: 'untracked', area: 'untracked' } + ]) + ).toBe(true) + expect( + shouldReloadDiffOnGitStatusChange(makeDiffFile({ diffSource: 'staged' }), [ + { path: 'file.ts', status: 'modified', area: 'staged' } + ]) + ).toBe(true) + }) }) diff --git a/src/renderer/src/components/editor/editor-panel-diff-reload.ts b/src/renderer/src/components/editor/editor-panel-diff-reload.ts index 0f7ec409e..ed33a5cbf 100644 --- a/src/renderer/src/components/editor/editor-panel-diff-reload.ts +++ b/src/renderer/src/components/editor/editor-panel-diff-reload.ts @@ -1,4 +1,5 @@ import type { OpenFile } from '@/store/slices/editor' +import type { GitStatusEntry } from '../../../../shared/types' export function isReloadableSingleFileDiffTab(file: OpenFile): boolean { return ( @@ -10,6 +11,40 @@ export function isReloadableSingleFileDiffTab(file: OpenFile): boolean { ) } -export function shouldReloadDiffOnGitStatusChange(file: OpenFile): boolean { - return file.mode === 'diff' && (file.diffSource === 'unstaged' || file.diffSource === 'staged') +function hasReloadableStatusEntry( + file: OpenFile, + gitStatusEntries: readonly GitStatusEntry[] | undefined +): boolean { + if (gitStatusEntries === undefined) { + return true + } + + // Why: a diff tab snapshots one status area. If staging/commit moves that + // row elsewhere, auto-reloading replaces useful context with another diff. + if (file.diffSource === 'unstaged') { + return gitStatusEntries.some( + (entry) => + entry.path === file.relativePath && + (entry.area === 'unstaged' || entry.area === 'untracked') + ) + } + + if (file.diffSource === 'staged') { + return gitStatusEntries.some( + (entry) => entry.path === file.relativePath && entry.area === 'staged' + ) + } + + return true +} + +export function shouldReloadDiffOnGitStatusChange( + file: OpenFile, + gitStatusEntries?: readonly GitStatusEntry[] +): boolean { + return ( + file.mode === 'diff' && + (file.diffSource === 'unstaged' || file.diffSource === 'staged') && + hasReloadableStatusEntry(file, gitStatusEntries) + ) } diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx index ec307769d..4244832ed 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx @@ -4,10 +4,12 @@ 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 type { FileContent } from './editor-panel-content-types' +import type { GitStatusEntry } from '../../../../shared/types' +import type { DiffContent, FileContent } from './editor-panel-content-types' const mocks = vi.hoisted(() => ({ readRuntimeFileContent: vi.fn(), + getRuntimeGitDiff: vi.fn(), getConnectionId: vi.fn(), getState: vi.fn() })) @@ -26,7 +28,7 @@ vi.mock('@/runtime/runtime-file-client', () => ({ vi.mock('@/runtime/runtime-git-client', () => ({ getRuntimeGitBranchDiff: vi.fn(), getRuntimeGitCommitDiff: vi.fn(), - getRuntimeGitDiff: vi.fn(), + getRuntimeGitDiff: mocks.getRuntimeGitDiff, getRuntimeGitScope: vi.fn(() => null) })) @@ -45,18 +47,27 @@ import { useEditorPanelContentState } from './useEditorPanelContentState' type ProbeProps = { activeFile: OpenFile openFiles: OpenFile[] + gitStatusByWorktree?: Record } let latestFileContents: Record = {} +let latestDiffContents: Record = {} +const EMPTY_GIT_STATUS_BY_WORKTREE: Record = {} -function HookProbe({ activeFile, openFiles }: ProbeProps): null { - latestFileContents = useEditorPanelContentState({ +function HookProbe({ + activeFile, + openFiles, + gitStatusByWorktree = EMPTY_GIT_STATUS_BY_WORKTREE +}: ProbeProps): null { + const state = useEditorPanelContentState({ activeFile, isChangesMode: false, openFiles, - gitStatusByWorktree: {}, + gitStatusByWorktree, editorViewMode: {} - }).fileContents + }) + latestFileContents = state.fileContents + latestDiffContents = state.diffContents return null } @@ -79,7 +90,9 @@ describe('useEditorPanelContentState', () => { beforeEach(() => { latestFileContents = {} + latestDiffContents = {} mocks.readRuntimeFileContent.mockReset() + mocks.getRuntimeGitDiff.mockReset() mocks.getConnectionId.mockReset() mocks.getConnectionId.mockReturnValue(undefined) mocks.getState.mockReset() @@ -126,4 +139,105 @@ describe('useEditorPanelContentState', () => { }) ) }) + + it('keeps a loaded unstaged diff when git status moves the row to staged', async () => { + const activeFile = createOpenFile({ + id: 'wt-1::diff::unstaged::file.ts', + mode: 'diff', + diffSource: 'unstaged' + }) + mocks.getRuntimeGitDiff.mockResolvedValue({ + kind: 'text', + originalContent: 'old', + modifiedContent: 'large diff content', + originalIsBinary: false, + modifiedIsBinary: false + }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + + await act(async () => { + root?.render( + + ) + }) + + await vi.waitFor(() => + expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('large diff content') + ) + + await act(async () => { + root?.render( + + ) + }) + + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(1) + }) + + it('reloads a loaded unstaged diff when its own status row is still present', async () => { + const activeFile = createOpenFile({ + id: 'wt-1::diff::unstaged::file.ts', + mode: 'diff', + diffSource: 'unstaged' + }) + mocks.getRuntimeGitDiff + .mockResolvedValueOnce({ + kind: 'text', + originalContent: 'old', + modifiedContent: 'first diff content', + originalIsBinary: false, + modifiedIsBinary: false + }) + .mockResolvedValueOnce({ + kind: 'text', + originalContent: 'old', + modifiedContent: 'refreshed diff content', + originalIsBinary: false, + modifiedIsBinary: false + }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + + await act(async () => { + root?.render() + }) + + await vi.waitFor(() => + expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('first diff content') + ) + + await act(async () => { + root?.render( + + ) + }) + + await vi.waitFor(() => + expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('refreshed diff content') + ) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2) + }) }) diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index 862e1aadf..acfeb72cb 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -336,19 +336,31 @@ export function useEditorPanelContentState({ const changesStatusEntries = activeFile?.worktreeId ? gitStatusByWorktree[activeFile.worktreeId] : undefined - const activeFileGitStatusSignature = useMemo(() => { + const activeFileGitStatusEntries = useMemo(() => { if (!activeFile?.relativePath || !changesStatusEntries) { + return undefined + } + return changesStatusEntries.filter((entry) => entry.path === activeFile.relativePath) + }, [activeFile?.relativePath, changesStatusEntries]) + const activeFileGitStatusSignature = useMemo(() => { + if (!activeFileGitStatusEntries) { return '' } - const matching = changesStatusEntries.filter((entry) => entry.path === activeFile.relativePath) return JSON.stringify( - matching.map((entry) => ({ + activeFileGitStatusEntries.map((entry) => ({ area: entry.area, status: entry.status, conflictStatus: entry.conflictStatus })) ) - }, [activeFile?.relativePath, changesStatusEntries]) + }, [activeFileGitStatusEntries]) + const activeFileShouldReloadOnGitStatusChange = useMemo( + () => + activeFile + ? shouldReloadDiffOnGitStatusChange(activeFile, activeFileGitStatusEntries) + : false, + [activeFile, activeFileGitStatusEntries] + ) useEffect(() => { if (!activeFile?.id) { return @@ -357,7 +369,7 @@ export function useEditorPanelContentState({ if (!current) { return } - if (!(isChangesMode || shouldReloadDiffOnGitStatusChange(current))) { + if (!(isChangesMode || activeFileShouldReloadOnGitStatusChange)) { return } // Why: the lazy-load effect already fetches on first open; forcing here @@ -366,7 +378,13 @@ export function useEditorPanelContentState({ return } void loadDiffContent(current, { force: true }) - }, [activeFileGitStatusSignature, isChangesMode, activeFile?.id, loadDiffContent]) + }, [ + activeFileShouldReloadOnGitStatusChange, + activeFileGitStatusSignature, + isChangesMode, + activeFile?.id, + loadDiffContent + ]) useEffect(() => { const nonce = activeFile?.diffContentReloadNonce