From 33d06baba6c16bfd58b541dfbe44d6651aaa1989 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 10 Jul 2026 03:06:44 -0700 Subject: [PATCH] Revert "Fix remote host branch diff routing (#6747)" This reverts commit 506f6ffbb9b970293af11895afdf278152b5a9e0. --- .../components/editor/CombinedDiffViewer.tsx | 9 +- .../combined-diff-section-connection.test.ts | 128 ------------------ .../combined-diff-section-connection.ts | 17 --- .../useEditorPanelContentState.test.tsx | 58 +------- .../editor/useEditorPanelContentState.ts | 8 +- .../src/lib/connection-context.test.ts | 40 ------ 6 files changed, 9 insertions(+), 251 deletions(-) delete mode 100644 src/renderer/src/components/editor/combined-diff-section-connection.test.ts delete mode 100644 src/renderer/src/components/editor/combined-diff-section-connection.ts diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index 8f9eee91a8e..a206fa8c8f9 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -18,8 +18,7 @@ import { createProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks import { joinPath } from '@/lib/path' import { detectLanguage } from '@/lib/language-detect' import { setWithLRU } from '@/lib/scroll-cache' -import { getConnectionIdForFile } from '@/lib/connection-context' -import { getCombinedDiffSectionConnectionId } from './combined-diff-section-connection' +import { getConnectionId, getConnectionIdForFile } from '@/lib/connection-context' import { findWorktreeById } from '@/store/slices/worktree-helpers' import { writeRuntimeFile } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' @@ -623,11 +622,7 @@ export default function CombinedDiffViewer({ let result: GitDiffResult let error: string | undefined try { - const connectionId = getCombinedDiffSectionConnectionId( - file.worktreeId, - file.filePath, - entry.path - ) + const connectionId = getConnectionId(file.worktreeId) ?? undefined const state = useAppStore.getState() const fileSettings = settingsForRuntimeOwner(state.settings, file.runtimeEnvironmentId) if ((isBranchMode || (isAllMode && !('area' in entry))) && branchCompare) { diff --git a/src/renderer/src/components/editor/combined-diff-section-connection.test.ts b/src/renderer/src/components/editor/combined-diff-section-connection.test.ts deleted file mode 100644 index 894f6df7830..00000000000 --- a/src/renderer/src/components/editor/combined-diff-section-connection.test.ts +++ /dev/null @@ -1,128 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest' -import type { FolderWorkspace, ProjectGroup, Repo } from '../../../../shared/types' -import { useAppStore } from '@/store' -import { folderWorkspaceKey } from '../../../../shared/workspace-scope' -import { getCombinedDiffSectionConnectionId } from './combined-diff-section-connection' - -const initialState = useAppStore.getInitialState() - -function makeRepo(overrides: Partial & { id: string }): Repo { - return { - path: '/home/neil/repo', - displayName: 'repo', - badgeColor: '#000', - addedAt: 0, - ...overrides - } -} - -function makeFolderWorkspace(folderPath: string): FolderWorkspace { - return { - id: 'folder-workspace-1', - projectGroupId: 'group-1', - name: 'Platform workspace', - folderPath, - linkedTask: null, - comment: '', - isArchived: false, - isUnread: false, - isPinned: false, - sortOrder: 1, - lastActivityAt: 0, - createdAt: 1, - updatedAt: 1 - } -} - -function makeGroup(parentPath: string): ProjectGroup { - return { - id: 'group-1', - name: 'Platform', - parentPath, - parentGroupId: null, - createdFrom: 'folder-scan', - tabOrder: 0, - isCollapsed: false, - color: null, - createdAt: 1, - updatedAt: 1 - } -} - -describe('getCombinedDiffSectionConnectionId', () => { - afterEach(() => { - useAppStore.setState(initialState, true) - }) - - it('routes a section to its SSH child repo in a mixed folder workspace (#6688)', () => { - const workspaceKey = folderWorkspaceKey('folder-workspace-1') - useAppStore.setState({ - folderWorkspaces: [makeFolderWorkspace('/home/neil/platform')], - projectGroups: [makeGroup('/home/neil/platform')], - repos: [ - makeRepo({ id: 'repo-local', path: '/home/neil/platform/web', projectGroupId: 'group-1' }), - makeRepo({ - id: 'repo-ssh', - path: '/home/neil/platform/api', - projectGroupId: 'group-1', - connectionId: 'ssh-1' - }) - ], - worktreesByRepo: {} - }) - - // Section paths are repo-tree-relative; joined onto the workspace root they - // must resolve to the owning child repo, not the ambiguous workspace. - expect( - getCombinedDiffSectionConnectionId(workspaceKey, '/home/neil/platform', 'api/src/index.ts') - ).toBe('ssh-1') - // Local child repo -> no connection (local read). - expect( - getCombinedDiffSectionConnectionId(workspaceKey, '/home/neil/platform', 'web/src/index.ts') - ).toBeUndefined() - // A path owned by no single child repo stays ambiguous. - expect( - getCombinedDiffSectionConnectionId(workspaceKey, '/home/neil/platform', 'README.md') - ).toBeUndefined() - }) - - it('composes Windows section paths with the workspace root separator', () => { - const workspaceKey = folderWorkspaceKey('folder-workspace-1') - // A mixed local+SSH workspace makes the whole-workspace owner ambiguous, so - // resolution must fall through to composing the section path — the behavior - // this test guards. A single-SSH workspace would resolve before joinPath. - useAppStore.setState({ - folderWorkspaces: [makeFolderWorkspace('C:\\Users\\neil\\platform')], - projectGroups: [makeGroup('C:\\Users\\neil\\platform')], - repos: [ - makeRepo({ - id: 'repo-local', - path: 'C:\\Users\\neil\\platform\\web', - projectGroupId: 'group-1' - }), - makeRepo({ - id: 'repo-ssh', - path: 'C:\\Users\\neil\\platform\\api', - projectGroupId: 'group-1', - connectionId: 'ssh-1' - }) - ], - worktreesByRepo: {} - }) - - expect( - getCombinedDiffSectionConnectionId( - workspaceKey, - 'C:\\Users\\neil\\platform', - 'api/src/index.ts' - ) - ).toBe('ssh-1') - expect( - getCombinedDiffSectionConnectionId( - workspaceKey, - 'C:\\Users\\neil\\platform', - 'web/src/index.ts' - ) - ).toBeUndefined() - }) -}) diff --git a/src/renderer/src/components/editor/combined-diff-section-connection.ts b/src/renderer/src/components/editor/combined-diff-section-connection.ts deleted file mode 100644 index b330ba4c13a..00000000000 --- a/src/renderer/src/components/editor/combined-diff-section-connection.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { getConnectionIdForFile } from '@/lib/connection-context' -import { joinPath } from '@/lib/path' - -/** - * Resolve the SSH connectionId that owns a combined-diff section's file. - * - * Why: folder workspaces mix local and SSH child repos, so the workspace - * worktree is an ambiguous owner; the section must resolve its host by the - * concrete child-repo path or a remote read is misrouted locally (#6688). - */ -export function getCombinedDiffSectionConnectionId( - worktreeId: string | null, - worktreeRootPath: string, - sectionPath: string -): string | undefined { - return getConnectionIdForFile(worktreeId, joinPath(worktreeRootPath, sectionPath)) ?? undefined -} diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx index 90807e66cc7..15cd29adb44 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx @@ -10,7 +10,6 @@ import type { DiffContent, FileContent } from './editor-panel-content-types' const mocks = vi.hoisted(() => ({ readRuntimeFileContent: vi.fn(), getRuntimeGitDiff: vi.fn(), - getRuntimeGitBranchDiff: vi.fn(), getConnectionId: vi.fn(), getConnectionIdForFile: vi.fn(), isWorktreeConnectionResolved: vi.fn(() => true), @@ -29,7 +28,7 @@ vi.mock('@/runtime/runtime-file-client', () => ({ })) vi.mock('@/runtime/runtime-git-client', () => ({ - getRuntimeGitBranchDiff: mocks.getRuntimeGitBranchDiff, + getRuntimeGitBranchDiff: vi.fn(), getRuntimeGitCommitDiff: vi.fn(), getRuntimeGitDiff: mocks.getRuntimeGitDiff, getRuntimeGitScope: vi.fn(() => null) @@ -132,7 +131,6 @@ describe('useEditorPanelContentState', () => { latestDiffContents = {} mocks.readRuntimeFileContent.mockReset() mocks.getRuntimeGitDiff.mockReset() - mocks.getRuntimeGitBranchDiff.mockReset() mocks.getConnectionId.mockReset() mocks.getConnectionId.mockReturnValue(undefined) mocks.getConnectionIdForFile.mockReset() @@ -190,60 +188,6 @@ describe('useEditorPanelContentState', () => { ) }) - it('loads folder workspace branch diffs through the path-specific SSH connection', async () => { - const activeFile = createOpenFile({ - id: 'branch-diff', - filePath: '/home/neil/platform/api/src/file.ts', - relativePath: 'api/src/file.ts', - worktreeId: 'folder:folder-workspace-1', - mode: 'diff', - diffSource: 'branch', - branchCompare: { - baseRef: 'main', - compareRef: 'feature', - compareVersion: 'feature', - baseOid: 'base', - headOid: 'head', - mergeBase: 'merge-base' - } - }) - mocks.getConnectionIdForFile.mockReturnValue('ssh-1') - mocks.getRuntimeGitBranchDiff.mockResolvedValue({ - kind: 'text', - originalContent: 'old', - modifiedContent: 'remote branch diff', - 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('remote branch diff') - ) - expect(mocks.getConnectionIdForFile).toHaveBeenCalledWith( - 'folder:folder-workspace-1', - '/home/neil/platform/api/src/file.ts' - ) - expect(mocks.getRuntimeGitBranchDiff).toHaveBeenCalledWith( - expect.objectContaining({ - worktreeId: 'folder:folder-workspace-1', - worktreePath: '/home/neil/platform', - connectionId: 'ssh-1' - }), - expect.objectContaining({ - compare: expect.objectContaining({ mergeBase: 'merge-base' }), - filePath: 'api/src/file.ts' - }) - ) - }) - it('does not read locally while a remote host worktree owner is still hydrating (#6648)', async () => { const activeFile = createOpenFile({ filePath: '/home/user/project/src/index.ts', diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index f4f668a5a4c..ae05de95475 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -3,7 +3,11 @@ make the hook coordination harder to audit. */ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { OpenFile } from '@/store/slices/editor' -import { getConnectionIdForFile, isWorktreeConnectionResolved } from '@/lib/connection-context' +import { + getConnectionId, + getConnectionIdForFile, + isWorktreeConnectionResolved +} from '@/lib/connection-context' import { joinPath } from '@/lib/path' import { useAppStore } from '@/store' import { getDiskBaselineSignature } from './diff-content-signature' @@ -220,7 +224,7 @@ export function useEditorPanelContentState({ ? file.branchCompare : null const commitCompare = file.commitCompare?.commitOid ? file.commitCompare : null - const connectionId = getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined + const connectionId = getConnectionId(file.worktreeId) ?? undefined const activeSettings = useAppStore.getState().settings const fileSettings = settingsForRuntimeOwner(activeSettings, file.runtimeEnvironmentId) const gitScope = getRuntimeGitScope(fileSettings, connectionId) diff --git a/src/renderer/src/lib/connection-context.test.ts b/src/renderer/src/lib/connection-context.test.ts index 48d27ecf1e4..bef67faf05f 100644 --- a/src/renderer/src/lib/connection-context.test.ts +++ b/src/renderer/src/lib/connection-context.test.ts @@ -256,46 +256,6 @@ describe('getConnectionId', () => { expect(getConnectionIdForFile(workspaceKey, '/home/neil/platform/README.md')).toBeUndefined() }) - it('resolves folder workspace combined diff sections by child repo path', () => { - const workspaceKey = folderWorkspaceKey('folder-workspace-1') - useAppStore.setState({ - folderWorkspaces: [makeFolderWorkspace()], - projectGroups: [ - { - id: 'group-1', - name: 'Platform', - parentPath: '/home/neil/platform', - parentGroupId: null, - createdFrom: 'folder-scan', - tabOrder: 0, - isCollapsed: false, - color: null, - createdAt: 1, - updatedAt: 1 - } - ], - repos: [ - makeRepo({ - id: 'repo-local', - path: '/home/neil/platform/web', - projectGroupId: 'group-1' - }), - makeRepo({ - id: 'repo-ssh', - path: '/home/neil/platform/api', - projectGroupId: 'group-1', - connectionId: 'ssh-1' - }) - ], - worktreesByRepo: {} - }) - - expect(getConnectionIdForFile(workspaceKey, '/home/neil/platform')).toBeUndefined() - expect(getConnectionIdForFile(workspaceKey, '/home/neil/platform/api/src/index.ts')).toBe( - 'ssh-1' - ) - }) - it('keeps explicit folder workspace provenance isolated from unrelated same-path SSH repos', () => { useAppStore.setState({ folderWorkspaces: [