fix(explorer): preserve opened folder links across refreshes (#32587)

With automatic symlink following disabled, re-reading a parent lists its links as files and discards the directory flag set when a user opened one. Keep that flag while a successful same-owner child listing is cached, in both direct reads and expanded-directory refreshes.

Mark inferred folder rows separately so a failed child read can clear them without changing host-confirmed directories. Preserve normal deleted-link and missing-target recovery behavior. Add parent/full refresh, failed read, ownership and Windows drive-root regression coverage.

Refs #29069.

Co-authored-by: Arnobrizwan <147672066+Arnobrizwan@users.noreply.github.com>
This commit is contained in:
Neil
2026-10-23 19:21:48 -07:00
committed by GitHub
co-authored by Arnobrizwan
parent b6d1f63e01
commit 1c0acb65fe
7 changed files with 297 additions and 14 deletions
@@ -10,7 +10,12 @@ vi.mock('./file-explorer-operation-owner', () => ({
getFileExplorerOwnerUnresolvedMessage: () => 'unresolved'
}))
import { readFileExplorerDirectory } from './file-explorer-directory-listing'
import {
keepOpenedDirectoryLinks,
clearFailedOpenedDirectoryLink,
readFileExplorerDirectory
} from './file-explorer-directory-listing'
import type { DirCache, TreeNode } from './file-explorer-types'
describe('readFileExplorerDirectory', () => {
it('re-sorts backend order — remote-runtime and paired-web routes return the host order verbatim', async () => {
@@ -39,3 +44,114 @@ it('preserves the host directory classification with default Quick Open preferen
expect(entries).toEqual([{ name: 'linked', isDirectory: true, isSymlink: true }])
expect(readRuntimeDirectory).toHaveBeenLastCalledWith(expect.anything(), '/w', undefined)
})
describe('keepOpenedDirectoryLinks', () => {
const node = (name: string, overrides: Partial<TreeNode> = {}): TreeNode => ({
name,
path: `/w/${name}`,
relativePath: name,
isDirectory: false,
depth: 0,
operationOwner: { kind: 'local' },
...overrides
})
it('keeps an opened folder link folder-shaped when the parent re-read lists it as a file', () => {
const previous = [node('linked', { isSymlink: true, isDirectory: true }), node('other')]
const next = [node('linked', { isSymlink: true }), node('other')]
const kept = keepOpenedDirectoryLinks(previous, next, {
'/w/linked': { children: [], operationOwner: { kind: 'local' } }
})
expect(kept.map((child) => [child.name, child.isDirectory])).toEqual([
['linked', true],
['other', false]
])
})
it('lets a link revert once its own listing is no longer cached', () => {
const previous = [node('linked', { isSymlink: true, isDirectory: true })]
const next = [node('linked', { isSymlink: true })]
expect(keepOpenedDirectoryLinks(previous, next, {})).toBe(next)
})
it('never promotes a link that was not opened, or a plain file with the same path', () => {
const previous = [node('linked', { isSymlink: true }), node('file', { isDirectory: true })]
const next = [node('linked', { isSymlink: true }), node('file')]
const cache = { '/w/linked': { children: [] }, '/w/file': { children: [] } }
expect(keepOpenedDirectoryLinks(previous, next, cache)).toBe(next)
})
it('does not restore a deleted link or a link replaced by a regular file', () => {
const previous = [node('linked', { isSymlink: true, isDirectory: true })]
const cache = { '/w/linked': { children: [], operationOwner: { kind: 'local' as const } } }
expect(keepOpenedDirectoryLinks(previous, [], cache)).toEqual([])
expect(keepOpenedDirectoryLinks(previous, [node('linked')], cache)[0].isDirectory).toBe(false)
})
it('does not reuse a failed or still-unproven child listing', () => {
const previous = [node('linked', { isSymlink: true, isDirectory: true })]
const next = [node('linked', { isSymlink: true })]
expect(keepOpenedDirectoryLinks(previous, next, { '/w/linked': { children: [] } })).toBe(next)
expect(
keepOpenedDirectoryLinks(previous, next, {
'/w/linked': { children: [], operationOwner: { kind: 'local' }, error: 'Not a directory' }
})
).toBe(next)
})
it('does not carry directory classification across operation owners', () => {
const previous = [node('linked', { isSymlink: true, isDirectory: true })]
const localCache = {
'/w/linked': { children: [], operationOwner: { kind: 'local' as const } }
}
const remoteNode = node('linked', {
isSymlink: true,
operationOwner: { kind: 'ssh', connectionId: 'server-2' }
})
expect(keepOpenedDirectoryLinks(previous, [remoteNode], localCache)[0].isDirectory).toBe(false)
const next = [node('linked', { isSymlink: true })]
expect(
keepOpenedDirectoryLinks(previous, next, {
'/w/linked': { children: [], operationOwner: remoteNode.operationOwner }
})
).toBe(next)
})
it('leaves unrelated unowned links untouched while preserving a known folder', () => {
const previous = [node('linked', { isSymlink: true, isDirectory: true })]
const next = [
node('linked', { isSymlink: true }),
node('unowned', { isSymlink: true, operationOwner: undefined })
]
const result = keepOpenedDirectoryLinks(previous, next, {
'/w/linked': { children: [], operationOwner: { kind: 'local' } }
})
expect(result.map((child) => child.isDirectory)).toEqual([true, false])
})
})
describe('clearFailedOpenedDirectoryLink', () => {
it.each(['/w/link', 'C:\\link'])('drops only inferred folder state for failed %s', (linkPath) => {
const parentPath = linkPath.startsWith('C:') ? 'C:\\' : '/w'
const cache: Record<string, DirCache> = {
[parentPath]: {
children: [
{
name: 'link',
path: linkPath,
relativePath: 'link',
isDirectory: true,
isSymlink: true,
isDirectoryFromOpenedLink: true,
depth: 0
}
]
}
}
expect(
clearFailedOpenedDirectoryLink(cache, linkPath)[parentPath].children[0].isDirectory
).toBe(false)
cache[parentPath].children[0].isDirectoryFromOpenedLink = false
expect(clearFailedOpenedDirectoryLink(cache, linkPath)).toBe(cache)
})
})
@@ -3,8 +3,9 @@ import { getRelativePathInsideRoot, joinPath } from '@/lib/path'
import type { DirEntry } from '../../../../shared/filesystem-entry-types'
import { sortDirEntries } from '../../../../shared/file-name-sort'
import { readRuntimeDirectory } from '@/runtime/runtime-file-mutation-client'
import type { FileExplorerOperationOwner, TreeNode } from './file-explorer-types'
import type { DirCache, FileExplorerOperationOwner, TreeNode } from './file-explorer-types'
import { shouldIncludeFileExplorerEntry } from './file-explorer-entries'
import { parentDirForWatchPath, resolveCachedDirPath } from './file-explorer-watch-path'
import {
getFileExplorerOperationOwner,
getFileExplorerOwnerUnresolvedMessage,
@@ -37,6 +38,61 @@ export function fileExplorerEntriesToTreeNodes(
})
}
// Without followSymlinks, re-reading a parent lists opened folder links as files (#29069).
export function keepOpenedDirectoryLinks(
previousChildren: readonly TreeNode[] | undefined,
nextChildren: TreeNode[],
dirCache: Readonly<Record<string, DirCache>>
): TreeNode[] {
const openedLinks = new Map<string, string>()
for (const child of previousChildren ?? []) {
const cachedLink = dirCache[child.path]
// Why: pending/failed reads and listings from another host aren't proof of an opened folder.
if (
child.isSymlink &&
child.isDirectory &&
child.operationOwner &&
child.operationOwner.kind !== 'unresolved' &&
cachedLink?.operationOwner &&
cachedLink.error === undefined &&
JSON.stringify(child.operationOwner) === JSON.stringify(cachedLink.operationOwner)
) {
openedLinks.set(child.path, JSON.stringify(child.operationOwner))
}
}
if (openedLinks.size === 0) {
return nextChildren
}
return nextChildren.map((child) =>
child.isSymlink &&
!child.isDirectory &&
child.operationOwner !== undefined &&
openedLinks.get(child.path) === JSON.stringify(child.operationOwner)
? { ...child, isDirectory: true, isDirectoryFromOpenedLink: true }
: child
)
}
export function clearFailedOpenedDirectoryLink(
cache: Record<string, DirCache>,
linkPath: string
): Record<string, DirCache> {
const parentPath = resolveCachedDirPath(cache, parentDirForWatchPath(linkPath))
if (!parentPath) {
return cache
}
const parent = cache[parentPath]
let changed = false
const children = parent.children.map((child) => {
if (child.path !== linkPath || !child.isSymlink || !child.isDirectoryFromOpenedLink) {
return child
}
changed = true
return { ...child, isDirectory: false, isDirectoryFromOpenedLink: false }
})
return changed ? { ...cache, [parentPath]: { ...parent, children } } : cache
}
export async function readFileExplorerDirectory(
activeWorktreeId: string | null | undefined,
worktreePath: string | null,
@@ -518,4 +518,46 @@ describe('refreshFileExplorerExpandedDirs', () => {
expect(readDirectory).toHaveBeenCalledTimes(1)
expect(cache['/repo/b']).toEqual(newerBCache)
})
it('keeps an opened folder link folder-shaped when a refresh re-reads its parent (#29069)', async () => {
let cache: Record<string, DirCache> = {
'/repo/src': {
children: [
{
name: 'linked',
path: '/repo/src/linked',
relativePath: 'src/linked',
isDirectory: true,
isSymlink: true,
depth: 1,
operationOwner: { kind: 'local' }
}
],
operationOwner: { kind: 'local' }
},
'/repo/src/linked': { children: [], operationOwner: { kind: 'local' } }
}
const setDirCache = vi.fn((update: CacheUpdate) => {
cache = typeof update === 'function' ? update(cache) : update
})
const { updateLoadingDirPaths } = createLoadingDirPathsRecorder()
await refreshFileExplorerExpandedDirs({
dirs: [{ dirPath: '/repo/src', depth: 0 }],
worktreePath: '/repo',
dirLoadTracker: createFileExplorerDirLoadTracker(),
setDirCache,
updateLoadingDirPaths,
readDirectory: async () => ({
// Without followSymlinks the host reports the link as a file.
entries: [{ name: 'linked', isDirectory: false, isSymlink: true }],
operationOwner: { kind: 'local' as const }
}),
maxConcurrentReads: 4
})
expect(cache['/repo/src'].children).toMatchObject([
{ name: 'linked', isDirectory: true, isSymlink: true }
])
})
})
@@ -3,6 +3,8 @@ import type { DirCache } from './file-explorer-types'
import type { FileExplorerDirLoadTracker } from './file-explorer-dir-load-tracker'
import {
fileExplorerEntriesToTreeNodes,
keepOpenedDirectoryLinks,
clearFailedOpenedDirectoryLink,
type FileExplorerDirectoryListing
} from './file-explorer-directory-listing'
import { forEachWithConcurrency } from '../../../../shared/map-with-concurrency'
@@ -96,7 +98,7 @@ export async function refreshFileExplorerExpandedDirs({
}
setDirCache((prev) => {
const next = { ...prev }
let next = { ...prev }
for (const result of currentResults) {
next[result.dirPath] =
result.cache.error !== undefined
@@ -105,7 +107,20 @@ export async function refreshFileExplorerExpandedDirs({
children: prev[result.dirPath]?.children ?? [],
error: result.cache.error
}
: result.cache
: {
...result.cache,
children: keepOpenedDirectoryLinks(
prev[result.dirPath]?.children,
result.cache.children,
prev
)
}
}
// Why: a parent may have preserved a link before this wave learned its target is gone.
for (const result of currentResults) {
if (result.cache.error !== undefined) {
next = clearFailedOpenedDirectoryLink(next, result.dirPath)
}
}
return next
})
@@ -136,3 +136,41 @@ it('a cached link recovers when its missing target directory returns', async ()
expect(hook.result.current.dirCache['/repo/link'].children[0].name).toBe('fresh.ts')
cleanup()
})
it('keeps a manually opened folder link after parent and full-tree refreshes (#29069)', async () => {
readDirectoryMock
.mockReset()
.mockImplementation(async (_id, _root, path) =>
path === '/repo'
? listing({ name: 'link', isDirectory: false, isSymlink: true })
: listing(entry('example.txt'))
)
const hook = renderHook(() => useFileExplorerTree('/repo', new Set(['/repo/link']), 'wt-1'))
await act(async () => {
await hook.result.current.loadDir('/repo', -1)
await hook.result.current.loadDir('/repo/link', 0, { force: true, failOnError: true })
hook.result.current.markPathAsDirectory('/repo/link')
})
expect(hook.result.current.rootCache?.children[0].isDirectory).toBe(true)
await act(async () => {
await hook.result.current.refreshDir('/repo')
})
expect(hook.result.current.rootCache?.children[0].isDirectory).toBe(true)
await act(async () => {
await hook.result.current.refreshTree()
})
expect(hook.result.current.rootCache?.children[0]).toMatchObject({
isDirectory: true,
isSymlink: true
})
expect(hook.result.current.dirCache['/repo/link'].children[0].name).toBe('example.txt')
readDirectoryMock.mockRejectedValueOnce(new Error('ENOTDIR'))
await act(async () => {
await hook.result.current.refreshDir('/repo/link')
})
expect(hook.result.current.rootCache?.children[0].isDirectory).toBe(false)
expect(hook.result.current.dirCache['/repo/link'].error).toBe('ENOTDIR')
})
@@ -12,6 +12,8 @@ export type TreeNode = {
relativePath: string
isDirectory: boolean
isSymlink?: boolean
/** Inferred from an opened link, not the parent's latest host listing. */
isDirectoryFromOpenedLink?: boolean
depth: number
/** Host snapshot that produced this node. Destructive actions fail closed without it. */
operationOwner?: FileExplorerOperationOwner
@@ -10,6 +10,8 @@ import {
} from './file-explorer-operation-owner'
import {
fileExplorerEntriesToTreeNodes,
keepOpenedDirectoryLinks,
clearFailedOpenedDirectoryLink,
readFileExplorerDirectory
} from './file-explorer-directory-listing'
import { refreshFileExplorerExpandedDirs } from './file-explorer-expanded-dirs-refresh'
@@ -135,7 +137,10 @@ export function useFileExplorerTree(
)
setDirCache((prev) => ({
...prev,
[dirPath]: { children, operationOwner: listing.operationOwner }
[dirPath]: {
children: keepOpenedDirectoryLinks(prev[dirPath]?.children, children, prev),
operationOwner: listing.operationOwner
}
}))
updateLoadingDirPaths((prev) => clearFileExplorerDirsLoading(prev, [dirPath]))
return true
@@ -152,14 +157,19 @@ export function useFileExplorerTree(
setSourceWorkspaceId(null)
rootReadFailedRef.current = true
}
setDirCache((prev) => ({
...prev,
[dirPath]: {
...prev[dirPath],
children: prev[dirPath]?.children ?? [],
error: describeFileExplorerReadError(error)
}
}))
setDirCache((prev) =>
clearFailedOpenedDirectoryLink(
{
...prev,
[dirPath]: {
...prev[dirPath],
children: prev[dirPath]?.children ?? [],
error: describeFileExplorerReadError(error)
}
},
dirPath
)
)
updateLoadingDirPaths((prev) => clearFileExplorerDirsLoading(prev, [dirPath]))
return !options?.failOnError
}
@@ -179,7 +189,11 @@ export function useFileExplorerTree(
}
changed = true
cacheChanged = true
return { ...child, isDirectory: true }
return {
...child,
isDirectory: true,
isDirectoryFromOpenedLink: child.isSymlink === true
}
})
next[dirPath] = cacheChanged ? { ...cache, children } : cache
}