fix: address Codex review on PR #2829

With an active search, ancestor keep decisions depend on cached
descendants. applyLocalMoveMutation only deleted source/target sorted
keys, leaving grandparent/root filter snapshots stale after Move To.

Co-authored-by: 陈大猫 <binaricat@users.noreply.github.com>

components/sftp/SftpPaneTreeView.tsx

@@ -9,7 +9,7 @@ import { useSftpPaneTreeRows } from './useSftpPaneTreeRows';
 import { SftpMoveToDialog } from './SftpMoveToDialog';
 import type { SftpFileEntry } from '../../types';
 import { getParentPath, isWindowsRoot, joinPath, resolveSftpWindowsPathOptions } from '../../application/state/sftp/utils';
-import { buildSftpColumnTemplate, filterHiddenFiles, isNavigableDirectory, isSftpColumnMenuKey, sortSftpEntries } from './utils';
+import { buildSftpColumnTemplate, filterHiddenFiles, filterSftpTreeEntriesByName, isNavigableDirectory, isSftpColumnMenuKey, sortSftpEntries } from './utils';
 import type { SftpTransferSource } from './SftpContext';
 import type { SftpPaneTreeViewProps } from './SftpPaneTreeView.types';
 import { sftpTreeSelectionStore, useSftpTreeSelectionState } from '../../application/state/sftp/sftpTreeSelectionStore';
@@ -225,12 +225,29 @@ export const SftpPaneTreeView = React.memo<SftpPaneTreeViewProps>(({
     childrenCacheRef.current.delete(targetPath);
     sortedChildrenCacheRef.current.delete(targetPath);
   }, []);
-  const prevSortKeyRef = useRef(`${sortField}:${sortOrder}:${directoriesFirst}:${pane.showHiddenFiles}`);
-  const sortKey = `${sortField}:${sortOrder}:${directoriesFirst}:${pane.showHiddenFiles}`;
+  const prevSortKeyRef = useRef(`${sortField}:${sortOrder}:${directoriesFirst}:${pane.showHiddenFiles}:${pane.filter}`);
+  const sortKey = `${sortField}:${sortOrder}:${directoriesFirst}:${pane.showHiddenFiles}:${pane.filter}`;
   if (prevSortKeyRef.current !== sortKey) {
     prevSortKeyRef.current = sortKey;
     sortedChildrenCacheRef.current.clear();
   }
+  // Ancestor keep decisions follow expand/load/error visibility; invalidate
+  // sorted snapshots when those states change which descendants the filter may use.
+  const prevExpandedPathsRef = useRef(expandedPaths);
+  if (prevExpandedPathsRef.current !== expandedPaths) {
+    prevExpandedPathsRef.current = expandedPaths;
+    sortedChildrenCacheRef.current.clear();
+  }
+  const prevLoadingPathsRef = useRef(loadingPaths);
+  if (prevLoadingPathsRef.current !== loadingPaths) {
+    prevLoadingPathsRef.current = loadingPaths;
+    sortedChildrenCacheRef.current.clear();
+  }
+  const prevErrorPathsRef = useRef(errorPaths);
+  if (prevErrorPathsRef.current !== errorPaths) {
+    prevErrorPathsRef.current = errorPaths;
+    sortedChildrenCacheRef.current.clear();
+  }
   useEffect(() => {
     const currentPath = pane.connection?.currentPath ?? '';
     if (!currentPath) {
@@ -253,6 +270,9 @@ export const SftpPaneTreeView = React.memo<SftpPaneTreeViewProps>(({
         return false;
       }
       childrenCacheRef.current.set(entryPath, children);
+      // Ancestor visibility depends on loaded descendants, so drop every
+      // sorted/filtered snapshot (not only this path) before the next row build.
+      sortedChildrenCacheRef.current.clear();
       dispatchTreePaths({ type: 'FINISH_LOADING', path: entryPath });
       return true;
     } catch {
@@ -481,7 +501,30 @@ export const SftpPaneTreeView = React.memo<SftpPaneTreeViewProps>(({
       const cached = sortedChildrenCacheRef.current.get(parentPath);
       if (cached) return cached;
       const sorted = sortSftpEntries(
-        filterHiddenFiles(entries, pane.showHiddenFiles),
+        filterSftpTreeEntriesByName(
+          filterHiddenFiles(entries, pane.showHiddenFiles),
+          pane.filter,
+          {
+            parentPath,
+            joinPath,
+            isDirectory: isNavigableDirectory,
+            getChildren: (entryPath) => {
+              // Match buildTree visibility: collapsed, loading, and error paths
+              // do not render children, so cached descendants must not keep
+              // ancestors either (reload/failure would otherwise leave a
+              // nonmatching parent with only a spinner/error row).
+              if (!expandedPaths.has(entryPath)) return undefined;
+              if (loadingPaths.has(entryPath) || errorPaths.has(entryPath)) {
+                return undefined;
+              }
+              const children = childrenCacheRef.current.get(entryPath);
+              if (!children) return undefined;
+              // Match visible-row hidden policy so ancestor keep decisions
+              // cannot latch onto descendants the user cannot see.
+              return filterHiddenFiles(children, pane.showHiddenFiles);
+            },
+          },
+        ),
         sortField,
         sortOrder,
         directoriesFirst,
@@ -521,6 +564,7 @@ export const SftpPaneTreeView = React.memo<SftpPaneTreeViewProps>(({
     resolvedRootPath,
     pane.connection?.homeDir,
     pane.showHiddenFiles,
+    pane.filter,
     sortField,
     sortOrder,
     directoriesFirst,
@@ -662,7 +706,6 @@ export const SftpPaneTreeView = React.memo<SftpPaneTreeViewProps>(({
         sourceParent,
         cached.filter((entry) => !movedNameSet.has(entry.name)),
       );
-      sortedChildrenCacheRef.current.delete(sourceParent);
     }
     if (targetPath !== currentPath) {
       const targetCache = childrenCacheRef.current.get(targetPath);
@@ -674,9 +717,12 @@ export const SftpPaneTreeView = React.memo<SftpPaneTreeViewProps>(({
           }
         }
         childrenCacheRef.current.set(targetPath, next);
-        sortedChildrenCacheRef.current.delete(targetPath);
       }
     }
+    // Ancestor filter keep decisions depend on cached descendants; drop every
+    // sorted/filtered snapshot (not only source/target) so Move To / drag-move
+    // cannot leave stale parents visible or hide the new parent under search.
+    sortedChildrenCacheRef.current.clear();
   }, [pane.connection?.currentPath]);
   const executeMoveAction = useCallback(async (sourcePaths: string[], targetPath: string) => {
     try {

components/sftp/hooks/useSftpPaneFiles.ts

@@ -3,7 +3,7 @@ import type { SftpFileEntry } from "../../../types";
 import type { SftpPane } from "../../../application/state/sftp/types";
 import { isWindowsRoot, resolveSftpWindowsPathOptions } from "../../../application/state/sftp/utils";
 import type { SortField, SortOrder } from "../utils";
-import { filterHiddenFiles, sortSftpEntries } from "../utils";
+import { filterHiddenFiles, filterSftpEntriesByName, sortSftpEntries } from "../utils";
 
 interface UseSftpPaneFilesParams {
   files: SftpFileEntry[];
@@ -36,11 +36,9 @@ export const useSftpPaneFiles = ({
   // in fewer passes, instead of repeatedly filtering/finding ".." entries.
   const filteredFiles = useMemo(() => {
     if (!enableListView) return [] as SftpFileEntry[];
-    const term = filter.trim().toLowerCase();
-    let nextFiles = filterHiddenFiles(files, showHiddenFiles);
-    if (!term) return nextFiles;
-    return nextFiles.filter(
-      (f) => f.name === ".." || f.name.toLowerCase().includes(term),
+    return filterSftpEntriesByName(
+      filterHiddenFiles(files, showHiddenFiles),
+      filter,
     );
   }, [enableListView, files, filter, showHiddenFiles]);
 

components/sftp/sftpNameFilter.test.ts

@@ -0,0 +1,240 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+
+import { filterSftpEntriesByName, filterSftpTreeEntriesByName } from './utils.ts';
+
+const entry = (name: string, type: 'file' | 'directory' = 'file') => ({ name, type });
+const isDirectory = (e: { type: string }) => e.type === 'directory';
+
+test('SFTP name filter returns all entries when term is empty', () => {
+  const files = [entry('..'), entry('README.md'), entry('src')];
+  assert.deepEqual(filterSftpEntriesByName(files, '   '), files);
+});
+
+test('SFTP name filter matches case-insensitively and keeps parent entry', () => {
+  const files = [entry('..'), entry('README.md'), entry('src'), entry('read-notes.txt')];
+  assert.deepEqual(
+    filterSftpEntriesByName(files, 'Read').map(({ name }) => name),
+    ['..', 'README.md', 'read-notes.txt'],
+  );
+});
+
+test('SFTP name filter hides non-matching siblings including directories', () => {
+  const files = [entry('config'), entry('logs'), entry('app.js')];
+  assert.deepEqual(
+    filterSftpEntriesByName(files, 'log').map(({ name }) => name),
+    ['logs'],
+  );
+});
+
+test('SFTP tree filter keeps loaded ancestors of matching children', () => {
+  const childrenByPath = new Map<string, ReturnType<typeof entry>[]>([
+    ['/project/src', [entry('README.md'), entry('utils.ts'), entry('components', 'directory')]],
+    ['/project/src/components', [entry('Button.tsx'), entry('readme-local.txt')]],
+  ]);
+  const root = [
+    entry('src', 'directory'),
+    entry('logs', 'directory'),
+    entry('app.js'),
+  ];
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren: (path) => childrenByPath.get(path),
+    }).map(({ name }) => name),
+    ['src'],
+  );
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(childrenByPath.get('/project/src')!, 'readme', {
+      parentPath: '/project/src',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren: (path) => childrenByPath.get(path),
+    }).map(({ name }) => name),
+    ['README.md', 'components'],
+  );
+});
+
+test('SFTP tree filter hides unloaded non-matching directories', () => {
+  const root = [entry('src', 'directory'), entry('README.md')];
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren: () => undefined,
+    }).map(({ name }) => name),
+    ['README.md'],
+  );
+});
+
+test('SFTP tree filter does not keep ancestors for hidden-only descendant matches', () => {
+  // getChildren must apply the same hidden-file policy as visible rows; otherwise a
+  // dotfile match can keep a parent folder while the matching child stays invisible.
+  const childrenByPath = new Map<string, ReturnType<typeof entry>[]>([
+    ['/project/src', [entry('.readme'), entry('utils.ts')]],
+  ]);
+  const root = [entry('src', 'directory')];
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren: (path) => {
+        const children = childrenByPath.get(path);
+        if (!children) return undefined;
+        return children.filter((child) => !child.name.startsWith('.'));
+      },
+    }).map(({ name }) => name),
+    [],
+  );
+});
+
+test('SFTP tree filter does not keep ancestors for collapsed descendant matches', () => {
+  // Collapsed directories keep children in cache, but buildTree does not render
+  // them. getChildren must treat collapsed paths as unavailable or the parent
+  // stays visible with no matching row.
+  const childrenByPath = new Map<string, ReturnType<typeof entry>[]>([
+    ['/project/src', [entry('README.md'), entry('utils.ts')]],
+  ]);
+  const expandedPaths = new Set<string>();
+  const root = [entry('src', 'directory'), entry('app.js')];
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren: (path) => {
+        if (!expandedPaths.has(path)) return undefined;
+        return childrenByPath.get(path);
+      },
+    }).map(({ name }) => name),
+    [],
+  );
+  expandedPaths.add('/project/src');
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren: (path) => {
+        if (!expandedPaths.has(path)) return undefined;
+        return childrenByPath.get(path);
+      },
+    }).map(({ name }) => name),
+    ['src'],
+  );
+});
+
+test('SFTP tree filter does not keep ancestors for loading or error descendant matches', () => {
+  // Expanded dirs keep children in cache during reload / after LOAD_ERROR, but
+  // buildTree only shows the loading or error row. getChildren must treat those
+  // paths as unavailable or a nonmatching parent stays as an empty result.
+  const childrenByPath = new Map<string, ReturnType<typeof entry>[]>([
+    ['/project/src', [entry('README.md'), entry('utils.ts')]],
+  ]);
+  const expandedPaths = new Set(['/project/src']);
+  const loadingPaths = new Set<string>();
+  const errorPaths = new Set<string>();
+  const root = [entry('src', 'directory'), entry('app.js')];
+  const getChildren = (path: string) => {
+    if (!expandedPaths.has(path)) return undefined;
+    if (loadingPaths.has(path) || errorPaths.has(path)) return undefined;
+    return childrenByPath.get(path);
+  };
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren,
+    }).map(({ name }) => name),
+    ['src'],
+  );
+  loadingPaths.add('/project/src');
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren,
+    }).map(({ name }) => name),
+    [],
+  );
+  loadingPaths.clear();
+  errorPaths.add('/project/src');
+  assert.deepEqual(
+    filterSftpTreeEntriesByName(root, 'readme', {
+      parentPath: '/project',
+      joinPath: (parent, name) => `${parent}/${name}`,
+      isDirectory,
+      getChildren,
+    }).map(({ name }) => name),
+    [],
+  );
+});
+
+test('SFTP tree view applies the tree name filter to visible rows', () => {
+  const treeSource = readFileSync(new URL('./SftpPaneTreeView.tsx', import.meta.url), 'utf8');
+  assert.match(
+    treeSource,
+    /sortSftpEntries\(\s*filterSftpTreeEntriesByName\(\s*filterHiddenFiles\(entries, pane\.showHiddenFiles\),\s*pane\.filter,/s,
+  );
+  assert.match(treeSource, /pane\.showHiddenFiles\}:\$\{pane\.filter\}/);
+  assert.match(
+    treeSource,
+    /getChildren:\s*\(entryPath\)\s*=>\s*\{[\s\S]*?expandedPaths\.has\(entryPath\)[\s\S]*?loadingPaths\.has\(entryPath\)[\s\S]*?errorPaths\.has\(entryPath\)[\s\S]*?filterHiddenFiles\([\s\S]*?pane\.showHiddenFiles/,
+  );
+  assert.match(
+    treeSource,
+    /prevExpandedPathsRef[\s\S]*?sortedChildrenCacheRef\.current\.clear\(\)/,
+  );
+  assert.match(
+    treeSource,
+    /prevLoadingPathsRef[\s\S]*?sortedChildrenCacheRef\.current\.clear\(\)/,
+  );
+  assert.match(
+    treeSource,
+    /prevErrorPathsRef[\s\S]*?sortedChildrenCacheRef\.current\.clear\(\)/,
+  );
+});
+
+test('SFTP tree child reload invalidates sorted cache so filter reapplies', () => {
+  // After expand/reload, childrenCache is replaced. Ancestor visibility also
+  // depends on those descendants, so the sorted cache must be cleared broadly
+  // (not only for the loaded path) or parents stay hidden.
+  const treeSource = readFileSync(new URL('./SftpPaneTreeView.tsx', import.meta.url), 'utf8');
+  const loadFn = treeSource.match(
+    /const loadChildrenForPath = useCallback\(async \(entryPath: string\) => \{[\s\S]*?\n {2}\}, \[\]\);/,
+  );
+  assert.ok(loadFn, 'expected loadChildrenForPath callback');
+  const setIdx = loadFn[0].indexOf('childrenCacheRef.current.set(entryPath, children)');
+  const clearIdx = loadFn[0].indexOf('sortedChildrenCacheRef.current.clear()');
+  assert.ok(setIdx >= 0, 'expected childrenCache write on successful load');
+  assert.ok(clearIdx > setIdx, 'sorted cache must clear after childrenCache write');
+});
+
+test('SFTP tree move mutation clears all sorted snapshots for ancestor filter', () => {
+  // With an active search, ancestor keep decisions depend on cached descendants.
+  // Move To / drag-move update childrenCache for source/target only; deleting just
+  // those sorted keys leaves grandparent/root snapshots stale (old parent kept,
+  // new parent hidden). Mirror loadChildrenForPath and clear every sorted snapshot.
+  const treeSource = readFileSync(new URL('./SftpPaneTreeView.tsx', import.meta.url), 'utf8');
+  const moveFn = treeSource.match(
+    /const applyLocalMoveMutation = useCallback\(\(\s*sourceParentPaths: string\[\],\s*targetPath: string,\s*movedEntries: SftpFileEntry\[\],\s*\) => \{[\s\S]*?\n {2}\}, \[pane\.connection\?\.currentPath\]\);/,
+  );
+  assert.ok(moveFn, 'expected applyLocalMoveMutation callback');
+  const body = moveFn[0];
+  assert.match(body, /childrenCacheRef\.current\.set\(/);
+  const lastChildrenSet = body.lastIndexOf('childrenCacheRef.current.set(');
+  const clearIdx = body.indexOf('sortedChildrenCacheRef.current.clear()');
+  assert.ok(clearIdx > lastChildrenSet, 'sorted cache must clear after childrenCache mutations');
+  assert.equal(
+    (body.match(/sortedChildrenCacheRef\.current\.delete\(/g) || []).length,
+    0,
+    'move mutation must not rely on per-path sorted deletes alone',
+  );
+});

components/sftp/utils.ts

@@ -347,3 +347,63 @@ export const filterHiddenFiles = <T extends { name: string; hidden?: boolean }>(
     if (showHiddenFiles) return files;
     return files.filter((f) => !isHiddenFile(f));
 };
+
+/**
+ * Filter files by search term (case-insensitive substring match on name).
+ * Always preserves ".." parent directory entry. Empty/whitespace terms are no-ops.
+ */
+export const filterSftpEntriesByName = <T extends { name: string }>(
+    files: T[],
+    filter: string,
+): T[] => {
+    const term = filter.trim().toLowerCase();
+    if (!term) return files;
+    return files.filter(
+        (f) => f.name === ".." || f.name.toLowerCase().includes(term),
+    );
+};
+
+export type SftpTreeNameFilterOptions<T extends { name: string }> = {
+    parentPath: string;
+    joinPath: (parentPath: string, name: string) => string;
+    isDirectory: (entry: T) => boolean;
+    /** Loaded children for a directory path; undefined means not loaded yet. */
+    getChildren: (entryPath: string) => T[] | undefined;
+};
+
+/**
+ * Tree-aware name filter: keeps list-view match rules, and also keeps directory
+ * ancestors when an expanded loaded descendant matches. Collapsed, loading,
+ * error, and unloaded directories only appear when their own name matches
+ * (callers should treat those paths as unavailable in getChildren; no
+ * server-side recursive search).
+ */
+export const filterSftpTreeEntriesByName = <T extends { name: string }>(
+    files: T[],
+    filter: string,
+    options: SftpTreeNameFilterOptions<T>,
+): T[] => {
+    const term = filter.trim().toLowerCase();
+    if (!term) return files;
+
+    const subtreeHasMatch = (entries: T[], parentPath: string): boolean => {
+        for (const entry of entries) {
+            if (entry.name === "..") continue;
+            if (entry.name.toLowerCase().includes(term)) return true;
+            if (!options.isDirectory(entry)) continue;
+            const entryPath = options.joinPath(parentPath, entry.name);
+            const children = options.getChildren(entryPath);
+            if (children && subtreeHasMatch(children, entryPath)) return true;
+        }
+        return false;
+    };
+
+    return files.filter((entry) => {
+        if (entry.name === "..") return true;
+        if (entry.name.toLowerCase().includes(term)) return true;
+        if (!options.isDirectory(entry)) return false;
+        const entryPath = options.joinPath(options.parentPath, entry.name);
+        const children = options.getChildren(entryPath);
+        return Boolean(children && subtreeHasMatch(children, entryPath));
+    });
+};