From 375a3e63510d82d0a09d01b7ef5f2ad33305ebfb Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 4 May 2026 00:01:04 -0700 Subject: [PATCH] feat(sidebar): path-aware filter in SSH remote file browser (#1384) Typing a path like `Documents/orca-internal` in the browse filter now resolves the path on the remote instead of producing "No matches". Supports `~`, absolute, and relative paths with live preview and Enter to navigate. Co-authored-by: Orca --- docs/ssh-folder-import-path-entry.md | 197 ++++++++ src/main/ipc/ssh-browse.ts | 25 + .../components/sidebar/RemoteFileBrowser.tsx | 466 ++++++++++++++++-- .../remote-file-browser-helpers.test.ts | 239 ++++++++- .../sidebar/remote-file-browser-helpers.ts | 163 ++++++ 5 files changed, 1038 insertions(+), 52 deletions(-) create mode 100644 docs/ssh-folder-import-path-entry.md diff --git a/docs/ssh-folder-import-path-entry.md b/docs/ssh-folder-import-path-entry.md new file mode 100644 index 00000000000..e22a1ef48d0 --- /dev/null +++ b/docs/ssh-folder-import-path-entry.md @@ -0,0 +1,197 @@ +# SSH Folder Picker — Path-Aware Filter + +## Problem + +On the "Browse remote filesystem" screen, the filter input only searches entries in the current directory. Users who know where they want to go, for example `Documents/orca-internal`, still have to click through each level manually. Typing a path with `/` currently produces "No matches" because the input treats it as a literal filter string. + +## Goal + +Let the user type a remote folder path like `Documents/orca-internal`, `~/Documents`, `/var/log`, or `../sibling` and navigate there from the existing filter input. The feature should preserve filter-only behavior for users who type ordinary names, avoid request storms over flaky SSH links, and fit the current `RemoteFileBrowser` model where `Select folder` returns the committed current directory. + +## Current Implementation Constraints + +The renderer entry point is `src/renderer/src/components/sidebar/RemoteFileBrowser.tsx`. Directory loading goes through `window.api.ssh.browseDir`, backed by `src/main/ipc/ssh-browse.ts`. + +The browse IPC currently accepts a `dirPath` string and returns `{ resolvedPath, entries }`, where each entry only has `{ name, isDirectory }`. It uses a remote shell command, `cd && pwd && ls -1ap`, not SFTP `readdir`/`stat`. The design below must therefore be implementable using repeated `browseDir` calls. Anything that requires symlink metadata, file type metadata beyond "directory or not", or a cancellable SSH command needs an explicit IPC contract change. + +## Design + +### Mode Switch + +The input has two modes: + +- **Filter mode**: ordinary text filters entries in the current directory. +- **Path mode**: path-like text resolves directory segments and uses the final partial segment as the filter in the resolved parent. + +Enter path mode when the input: + +- contains `/` +- starts with `~/`, `./`, or `../` +- equals `~`, `.`, or `..` + +The explicit `..` cases are required because "presence of `/`" alone would make bare `..` behave like a filter instead of parent navigation. + +### Input Parsing + +| Input | Meaning | +|---|---| +| `docs` | Filter current directory | +| `Documents/orca-internal` | Resolve `Documents`, then filter by `orca-internal` | +| `Documents/` | Resolve and show `Documents` with no filter | +| `/var/log` | Resolve from remote root | +| `~/Documents` | Resolve from the SSH user's home | +| `..` | Parent of current directory | +| `../sibling` | Parent of current directory, then filter by `sibling` | + +Parsing should preserve the raw input. Do not normalize away a trailing slash, because `Documents` and `Documents/` mean different things: the first stays in filter mode until Enter, while the second enters path mode and previews `Documents` with an empty filter. Typing a trailing slash does not by itself commit navigation; Enter, a row click, or a breadcrumb click are the only navigation commit actions. + +### Base Path + +Resolution starts from: + +- `/` for absolute inputs beginning with `/` +- the resolved home directory for inputs beginning with `~` +- the current `resolvedPath` for relative inputs + +`browseDir('~')` already resolves the remote user's home and returns the absolute `resolvedPath`. Cache that result, but do not hardcode a home path in the renderer. Treat `~` as a base marker, not as a directory name to match under the current directory. + +### Resolution Algorithm + +For a path-mode input: + +1. Split the input into a base, committed path segments, and a trailing filter segment. A segment is committed when it appears before the final separator or when the input is exactly `~`, `.`, or `..`. Ignore the empty segment created by one leading `/` for absolute paths and by one trailing `/`; other empty segments from repeated separators should produce an inline invalid-path error instead of silently rewriting the user input. +2. Resolve committed segments one at a time from the base path: + - `.` keeps the current base. + - `..` moves to the parent path. If already at `/`, stay at `/`. + - exact directory match descends. + - exact non-directory match stops resolution and shows an inline error. + - unique prefix match among directories descends. + - ambiguous prefix stops resolution and shows an inline error. + - no directory match stops resolution and shows an inline error. +3. Once committed segments resolve, display that directory's cached or fetched listing. +4. Apply the trailing segment as the local filter in that resolved directory. +5. Keep the raw input intact on errors. Do not clear user text unless navigation is committed by Enter, a row click, or a breadcrumb click. + +Path-mode typing must not call the existing `navigate(path)` wrapper directly. It should update separate preview state, for example `{ previewResolvedPath, previewEntries, previewFilter, previewError, previewLoading }`, while leaving the committed `resolvedPath` untouched. Otherwise typing `Documents/` would change the `Select folder` target before the user commits the path. All committed navigation still goes through `navigate(path)` so the committed current directory, breadcrumb, loading state, and `Select folder` target remain consistent. + +The list can render the preview listing while path mode is active, but the footer and `Select folder` target should make the committed path clear. A simple implementation is: + +- `Select folder` keeps today's behavior and selects the committed `resolvedPath` when the input is empty or in filter mode. +- `Select folder` is disabled while a non-empty path-mode preview is visible. This prevents silently selecting the old committed directory while the list is showing a different preview directory. +- A row click in a preview listing commits navigation relative to `previewResolvedPath`, not the old committed `resolvedPath`. + +### Enter Key + +In filter mode, keep today's behavior: + +- one matching folder navigates into it +- only file matches show the file hint +- ambiguous folder matches do nothing + +In path mode: + +- fully resolved directory, including a trailing `/`, navigates there and clears the input +- resolved parent plus one matching child folder navigates into that child and clears the input +- resolved parent plus one exact non-directory match shows the file hint or an inline "not a directory" error and does not clear the input +- ambiguous or invalid path keeps the input and shows the inline error + +### Backspace Out Of Empty Input + +Optional. If implemented, Backspace on an empty input navigates to the parent directory, equivalent to the breadcrumb up button. It should not fire when the caret is inside non-empty text. + +### Paste + +Pasting a path resolves through the same parser as typing. Treat a paste as one logical operation: start resolving immediately, show loading state, and drop stale results if the user edits before it completes. + +## Debouncing And Request Control + +### Filter Mode + +Filtering is local against the current `entries` array. Do not call `browseDir` for filter-only edits. A 60-100ms render debounce is acceptable for large directories, but it is not required for correctness. If directories can contain thousands of entries, list virtualization is the larger performance fix. + +### Path Mode + +Remote calls are only needed when a committed segment requires a listing that is not already cached. + +Rules: + +- Debounce typed path resolution by 250-350ms. +- Do not debounce paste. +- Track a monotonically increasing request id and ignore stale responses. `AbortController` alone is insufficient unless the IPC contract is changed to support cancellation. +- Cache directory listings by `targetId + absolute resolved path` for the lifetime of the picker. +- Reuse the already-loaded current directory listing as the first cache entry. +- Keep committed directory state and preview directory state separate. The existing `genRef` pattern in `RemoteFileBrowser` protects committed `loadDir` calls, but path preview needs its own request id so a stale preview cannot overwrite committed navigation after the user clicks a breadcrumb or row. +- Do not fetch for partial trailing segment changes. For `Documents/orc` to `Documents/orca`, `Documents` is already resolved, so only the local filter changes. +- Keep the previous visible listing while resolving the next directory. Show a subtle spinner in the input instead of flashing the list to empty. + +Invariant: ordinary typing should cause at most one uncached `browseDir` call per newly committed path segment. Paste may issue multiple sequential `browseDir` calls, one per uncached segment, because resolving `/home/neil/project` requires proving each intermediate directory. + +## Errors And Empty States + +Path-mode errors render below the input and do not replace the file list: + +- unresolved segment: `Documentz isn't a directory in /home/neil` +- ambiguous segment: `Doc matches multiple directories in /home/neil` +- permission denied: `Permission denied: /home/neil/private` + +The current `ssh:browseDir` implementation needs a small correctness fix for this to work reliably. It rejects only when `stderr` is present and `stdout` is empty, but `cd && pwd && ls -1ap` can print `pwd` to stdout and then fail `ls` with permission denied. In that case the handler currently looks like a successful empty directory. The handler should reject on non-zero exit status, or use a command shape that emits a machine-readable status for `ls`, before this PR claims permission-denied handling. + +Empty-state copy should distinguish filter emptiness from directory emptiness: + +- current directory has no entries: `Empty directory` +- path mode resolved to an empty directory: `/home/neil/Documents is empty` +- filter hides every entry: `No matches for 'orca'` + +## Edge Cases + +- **Symlinks to directories**: the current IPC cannot identify or follow them reliably while also exposing symlink metadata. Do not promise symlink-specific UI in this PR unless `ssh:browseDir` is changed to return richer entry metadata. +- **Case sensitivity**: the remote listing is authoritative. Exact (case-sensitive) match wins first so users with both `Documents` and `documents` get what they typed. When no case-sensitive match exists, fall back to a case-insensitive exact match, then a case-insensitive unique prefix match. Without this fallback, typing `documents/` errors while `documents` (no slash) finds `Documents` via the filter — the two modes must not disagree. +- **Trailing slash**: `foo/` commits `foo` as a path segment for preview resolution and shows that directory with an empty filter. It does not commit picker navigation until Enter or a row click. +- **Repeated separators**: reject `foo//bar` as invalid in path mode. Silently collapsing it would make the visible input disagree with the path being resolved. +- **Whitespace**: filter mode can continue trimming for search, but path mode must preserve spaces inside segments and should not trim the full input before parsing. Remote paths can legitimately begin or end with spaces. +- **Remote Windows paths**: the current browse command and this design are POSIX-path oriented. Do not add partial `C:\...` support in the renderer without first making `ssh:browseDir` shell/path handling Windows-aware. +- **Names containing `/`**: impossible to represent as path segments. Treat `/` as a separator. + +## Tests + +Add focused unit tests around a pure parser/resolver helper, then keep `RemoteFileBrowser` tests thin: + +- no slash stays in filter mode +- `..` enters path mode and resolves to parent +- `../sibling` resolves parent and filters by `sibling` +- `Documents/orca` resolves `Documents` and filters by `orca` +- `Documents/` previews `Documents` with an empty filter, and Enter navigates into it +- `/var/log` resolves from root +- `~/Documents` resolves from remote home +- `~` resolves and commits the remote home on Enter +- `./child` resolves from the committed current directory +- exact file match in a committed segment reports "not a directory" instead of descending by prefix +- repeated separators report an invalid-path error +- path-mode parsing preserves spaces in segments +- path preview does not change `resolvedPath` or the `Select folder` target before commit +- `Select folder` is disabled while a non-empty path preview is visible +- unique prefix descends +- ambiguous prefix reports an error and does not navigate +- missing segment reports an error and does not clear input +- permission denied from `ls` rejects instead of rendering an empty directory +- stale async resolution result is ignored after input changes +- stale async preview result is ignored after committed navigation +- cached directories are not fetched again +- partial trailing filter edits do not call `browseDir` +- paste resolves immediately and sequentially +- Enter in path mode clears input only after successful navigation +- existing filter-mode Enter behavior is unchanged + +## Files To Change + +- `src/renderer/src/components/sidebar/remote-file-browser-helpers.ts`: add parser and pure resolution decision helpers. +- `src/renderer/src/components/sidebar/remote-file-browser-helpers.test.ts`: expand coverage for path mode. +- `src/renderer/src/components/sidebar/RemoteFileBrowser.tsx`: wire path mode, cache, request ids, inline errors, loading affordance, and Enter behavior. +- `src/main/ipc/ssh-browse.ts`: fix error reporting so a failed `ls` after a successful `pwd` rejects instead of returning an empty listing. No metadata or cancellation contract change is required for the base path-entry feature. + +## Non-Goals + +- New UI controls such as a "go to path" button. +- Rich symlink display. +- Remote Windows path support beyond what the current SSH browse command already handles. +- Changing the picker selection model. `Select folder` continues to return the current resolved directory. diff --git a/src/main/ipc/ssh-browse.ts b/src/main/ipc/ssh-browse.ts index cbabe7db0f1..874e677dbd7 100644 --- a/src/main/ipc/ssh-browse.ts +++ b/src/main/ipc/ssh-browse.ts @@ -34,12 +34,16 @@ export function registerSshBrowseHandler( // filenames containing spaces or special characters. The -1 flag outputs // one entry per line. The -p flag appends / to directories. // We resolve ~ and get the absolute path via `cd && pwd`. + // `cd` and `ls` are chained with `&&` so a failing `ls` (e.g. permission + // denied after a readable `cd ... && pwd`) propagates as a non-zero exit + // code rather than being indistinguishable from an empty directory. const command = `cd ${shellEscape(args.dirPath)} && pwd && ls -1ap` const channel = await conn.exec(command) return new Promise((resolve, reject) => { let stdout = '' let stderr = '' + let exitCode: number | null = null channel.on('data', (data: Buffer) => { stdout += data.toString() @@ -47,7 +51,28 @@ export function registerSshBrowseHandler( channel.stderr.on('data', (data: Buffer) => { stderr += data.toString() }) + // `exit` fires before `close`; capture the code so we can distinguish + // a failed `ls` that still produced `pwd` output from an empty listing. + channel.on('exit', (code: number | null) => { + exitCode = code + }) channel.on('close', () => { + // A null exitCode means the server closed the channel without + // sending an exit-status message (or signalled termination). We + // can't assume success — falling back to "empty stdout = empty + // directory" is exactly the bug the exit-code branch was added to + // fix. Treat any non-zero OR null exit as a failure when stderr + // has content, and otherwise require stdout to contain at least + // the resolved `pwd` line before accepting the result. + if (exitCode !== 0) { + const msg = + stderr.trim() || + (exitCode === null + ? 'Remote listing failed (channel closed without exit status)' + : `Remote listing failed (exit ${exitCode})`) + reject(new Error(msg)) + return + } if (stderr.trim() && !stdout.trim()) { reject(new Error(stderr.trim())) return diff --git a/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx b/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx index fa54411e96c..889c66a7ec4 100644 --- a/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx +++ b/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- Why: the remote file browser centralizes filter state, path-mode preview state, cache, debounce, request gen, and click/keyboard handling in one component so picker navigation stays coherent. */ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ChevronRight, Folder, File, ArrowUp, LoaderCircle, Home, Search } from 'lucide-react' import { Button } from '@/components/ui/button' @@ -6,7 +7,11 @@ import { decideEnterAction, decideEscAction, filterEntries, + isPathMode, joinPath, + parentPath, + parsePathInput, + resolveSegmentStep, type DirEntry } from './remote-file-browser-helpers' @@ -19,6 +24,17 @@ type RemoteFileBrowserProps = { const FILE_HINT_MS = 2000 const FILE_HINT_TEXT = "Files can't be opened as a project" +const PATH_DEBOUNCE_MS = 300 + +type BrowseResult = { resolvedPath: string; entries: DirEntry[] } + +type PreviewState = { + resolvedPath: string + entries: DirEntry[] + filter: string + error: string | null + loading: boolean +} export function RemoteFileBrowser({ targetId, @@ -32,9 +48,26 @@ export function RemoteFileBrowser({ const [error, setError] = useState(null) const [filter, setFilter] = useState('') const [fileHint, setFileHint] = useState(false) + // Preview state drives the list while path mode is active. It is kept + // separate from committed state so typing `Documents/` does not silently + // change the `Select folder` target before the user commits. + const [preview, setPreview] = useState(null) const genRef = useRef(0) + const previewGenRef = useRef(0) const inputRef = useRef(null) const fileHintTimerRef = useRef | null>(null) + const debounceTimerRef = useRef | null>(null) + // Cache directory listings by absolute resolved path for the lifetime of + // the picker so ordinary typing issues at most one remote call per newly + // committed segment. targetId does not change within a picker instance. + const listingCacheRef = useRef>(new Map()) + // Resolved remote home, cached after the first `browseDir('~')`. Used to + // anchor `~` and `~/...` paths without hardcoding a home directory. + const homePathRef = useRef(null) + // The committed-path portion of the raw input that the current preview + // reflects (everything up to and including the final `/`). If the user's + // next keystroke leaves this unchanged, we can skip re-resolving. + const lastCommittedPrefixRef = useRef('') const clearFileHint = useCallback(() => { if (fileHintTimerRef.current) { @@ -49,21 +82,50 @@ export function RemoteFileBrowser({ if (fileHintTimerRef.current) { clearTimeout(fileHintTimerRef.current) } + if (debounceTimerRef.current) { + clearTimeout(debounceTimerRef.current) + } } }, []) + const fetchListing = useCallback( + async (dirPath: string): Promise => { + const cached = listingCacheRef.current.get(dirPath) + if (cached) { + return cached + } + const result = await window.api.ssh.browseDir({ targetId, dirPath }) + listingCacheRef.current.set(result.resolvedPath, result) + // Also cache under the requested dirPath when it differs from the + // server-resolved canonical path (e.g. `~`, `~/foo`, or a relative + // input). Without this, the next identical request would miss the + // cache and re-hit the SSH backend. + if (dirPath !== result.resolvedPath) { + listingCacheRef.current.set(dirPath, result) + } + return result + }, + [targetId] + ) + const loadDir = useCallback( async (dirPath: string) => { const gen = ++genRef.current setLoading(true) setError(null) try { - const result = await window.api.ssh.browseDir({ targetId, dirPath }) + const result = await fetchListing(dirPath) if (gen !== genRef.current) { return } setResolvedPath(result.resolvedPath) setEntries(result.entries) + // Only the bare-tilde listing returns the home directory itself; + // `~/sub` resolves to `.../sub`, which must not overwrite the home + // anchor used for resolving later `~/...` inputs. + if (dirPath === '~') { + homePathRef.current = result.resolvedPath + } } catch (err) { if (gen !== genRef.current) { return @@ -76,15 +138,23 @@ export function RemoteFileBrowser({ } } }, - [targetId] + [fetchListing] ) - // All user-initiated navigation goes through this wrapper so filter + hint - // state is always cleared. The initial mount calls loadDir directly so a - // user who types during the first load keeps their input. + // All user-initiated navigation goes through this wrapper so filter + + // preview + hint state is always cleared. Bumping previewGenRef here + // ensures any in-flight path preview whose target is no longer relevant + // can't overwrite committed state after a breadcrumb or row click. const navigate = useCallback( (dirPath: string) => { setFilter('') + setPreview(null) + previewGenRef.current++ + lastCommittedPrefixRef.current = '' + if (debounceTimerRef.current) { + clearTimeout(debounceTimerRef.current) + debounceTimerRef.current = null + } clearFileHint() loadDir(dirPath) }, @@ -106,12 +176,16 @@ export function RemoteFileBrowser({ if (resolvedPath === '/') { return } - const parent = resolvedPath.replace(/\/[^/]+\/?$/, '') || '/' - navigate(parent) + navigate(parentPath(resolvedPath)) }, [resolvedPath, navigate]) const filteredEntries = useMemo(() => filterEntries(entries, filter), [entries, filter]) + const previewFilteredEntries = useMemo( + () => (preview ? filterEntries(preview.entries, preview.filter) : []), + [preview] + ) + const triggerFileHint = useCallback(() => { if (fileHintTimerRef.current) { clearTimeout(fileHintTimerRef.current) @@ -123,18 +197,212 @@ export function RemoteFileBrowser({ }, FILE_HINT_MS) }, []) - // Select always returns the current directory. Selection model = "navigate - // to the folder you want, then Select"; this is the VS Code approach and - // was chosen after feedback that the highlight-a-row model was confusing. + // Resolve a path-mode input and push the result into preview state. + // Exposed as a ref-callback so it can run immediately on paste or on the + // debounce tick without re-creating on every keystroke. + const resolvePathInput = useCallback( + async (raw: string) => { + const parsed = parsePathInput(raw) + if (parsed.mode !== 'path') { + return + } + const gen = ++previewGenRef.current + + if (parsed.invalid) { + setPreview({ + resolvedPath: resolvedPath, + entries: [], + filter: '', + error: parsed.invalid, + loading: false + }) + return + } + + // Pick the base path. For `~` we must know the resolved home; if we + // haven't fetched it yet, fetch once (and cache it) before resolving. + let basePath: string + if (parsed.base === 'root') { + basePath = '/' + } else if (parsed.base === 'home') { + if (!homePathRef.current) { + setPreview({ + resolvedPath: resolvedPath, + entries: [], + filter: '', + error: null, + loading: true + }) + try { + const home = await fetchListing('~') + if (gen !== previewGenRef.current) { + return + } + homePathRef.current = home.resolvedPath + } catch (err) { + if (gen !== previewGenRef.current) { + return + } + setPreview({ + resolvedPath, + entries: [], + filter: '', + error: err instanceof Error ? err.message : String(err), + loading: false + }) + return + } + } + basePath = homePathRef.current! + } else { + basePath = resolvedPath + } + + setPreview((prev) => ({ + resolvedPath: prev?.resolvedPath ?? basePath, + entries: prev?.entries ?? [], + filter: prev?.filter ?? '', + error: null, + loading: true + })) + + let currentPath = basePath + try { + for (const segment of parsed.committedSegments) { + const listing = await fetchListing(currentPath) + if (gen !== previewGenRef.current) { + return + } + const outcome = resolveSegmentStep(segment, currentPath, listing.entries) + if (outcome.type === 'error') { + setPreview({ + resolvedPath: currentPath, + entries: listing.entries, + filter: '', + error: outcome.message, + loading: false + }) + return + } + if (outcome.type === 'stay') { + if (segment === '..') { + currentPath = parentPath(currentPath) + } + continue + } + currentPath = joinPath(currentPath, outcome.name) + } + + const finalListing = await fetchListing(currentPath) + if (gen !== previewGenRef.current) { + return + } + lastCommittedPrefixRef.current = committedPrefix(raw) + setPreview({ + resolvedPath: finalListing.resolvedPath, + entries: finalListing.entries, + filter: parsed.trailingFilter, + error: null, + loading: false + }) + } catch (err) { + if (gen !== previewGenRef.current) { + return + } + setPreview({ + resolvedPath: currentPath, + entries: [], + filter: '', + error: err instanceof Error ? err.message : String(err), + loading: false + }) + } + }, + [resolvedPath, fetchListing] + ) + + // Called on every user edit to the input. Filter-mode edits stay local; + // path-mode edits trigger a debounced resolve. Partial trailing-segment + // changes that don't change committed segments only update the preview + // filter, so typing `Documents/orc` → `Documents/orca` is free. + const handleInputChange = useCallback( + (raw: string) => { + clearFileHint() + setFilter(raw) + + if (!isPathMode(raw)) { + // Leaving path mode: drop preview immediately so the committed + // directory re-appears without a flicker. + if (preview) { + setPreview(null) + previewGenRef.current++ + } + if (debounceTimerRef.current) { + clearTimeout(debounceTimerRef.current) + debounceTimerRef.current = null + } + return + } + + const parsed = parsePathInput(raw) + // Partial trailing-segment edits: if the committed-path portion of the + // input is unchanged from what the preview already resolved, update + // only the local filter. This is the fast path that guarantees typing + // `Documents/orc` → `Documents/orca` issues no `browseDir` call. + if ( + parsed.mode === 'path' && + preview && + !preview.error && + !parsed.invalid && + committedPrefix(raw) === lastCommittedPrefixRef.current + ) { + // Intentionally allow this fast path to run even while + // preview.loading is true: the committed prefix is unchanged, so + // the in-flight resolve will land on the same listing and only the + // trailing filter needs updating. Blocking on loading would make + // keystrokes during a slow resolve feel unresponsive. + setPreview({ ...preview, filter: parsed.trailingFilter }) + return + } + + if (debounceTimerRef.current) { + clearTimeout(debounceTimerRef.current) + } + debounceTimerRef.current = setTimeout(() => { + debounceTimerRef.current = null + resolvePathInput(raw) + }, PATH_DEBOUNCE_MS) + }, + [clearFileHint, preview, resolvePathInput] + ) + + const handleInputPaste = useCallback( + (_e: React.ClipboardEvent) => { + // Paste resolves immediately; no debounce. React's onChange still fires + // after the paste is applied to the input value, so we defer to the + // next tick so `filter` reflects the pasted value. + setTimeout(() => { + if (debounceTimerRef.current) { + clearTimeout(debounceTimerRef.current) + debounceTimerRef.current = null + } + const value = inputRef.current?.value ?? '' + if (isPathMode(value)) { + resolvePathInput(value) + } + }, 0) + }, + [resolvePathInput] + ) + + // Select always returns the committed current directory. Disabled while a + // path-mode preview is visible so the user can't silently select the old + // committed directory while the list shows a different preview directory. const handleSelect = useCallback(() => { onSelect(resolvedPath) }, [resolvedPath, onSelect]) - // Single-click navigates; double-click on a folder selects it. Because - // onClick fires on both mousedowns of a dblclick, we have to delay the - // single-click so a subsequent dblclick can cancel it. 220ms matches the - // platform dblclick threshold on macOS closely enough that the single-click - // latency is imperceptible. + // Single-click navigates; double-click on a folder selects it. const clickTimerRef = useRef | null>(null) useEffect(() => { return () => { @@ -144,42 +412,81 @@ export function RemoteFileBrowser({ } }, []) + // When preview is active, row clicks must be relative to the preview path, + // not the committed `resolvedPath`. + const listParentPath = preview?.resolvedPath ?? resolvedPath + const handleRowClick = useCallback( (entry: DirEntry) => { + // Stale entries from the previous resolved listing can remain on + // screen while a new preview resolves; clicking them would navigate + // relative to a path that no longer matches what the user is typing. + if (preview?.loading) { + return + } if (clickTimerRef.current) { clearTimeout(clickTimerRef.current) } clickTimerRef.current = setTimeout(() => { clickTimerRef.current = null if (entry.isDirectory) { - navigateInto(entry.name) + navigate(joinPath(listParentPath, entry.name)) } else { - // Files aren't navigable and can't be opened as a project — the - // footer hint keeps the click from being a silent no-op. triggerFileHint() } }, 220) }, - [navigateInto, triggerFileHint] + [navigate, triggerFileHint, listParentPath, preview?.loading] ) const handleRowDoubleClick = useCallback( (entry: DirEntry) => { - if (!entry.isDirectory) { + // Same rationale as handleRowClick: do not act on stale rows while + // the preview listing is being re-resolved. + if (!entry.isDirectory || preview?.loading) { return } if (clickTimerRef.current) { clearTimeout(clickTimerRef.current) clickTimerRef.current = null } - onSelect(joinPath(resolvedPath, entry.name)) + onSelect(joinPath(listParentPath, entry.name)) }, - [resolvedPath, onSelect] + [listParentPath, onSelect, preview?.loading] ) const handleFilterKeyDown = useCallback( (e: React.KeyboardEvent) => { if (e.key === 'Enter') { + if (preview) { + // Path mode Enter. + if (preview.error || preview.loading) { + e.preventDefault() + return + } + const parsed = parsePathInput(filter) + // Fully-resolved directory (trailing `/` or bare base marker): + // navigate to the preview path itself. + if (parsed.mode === 'path' && parsed.trailingFilter === '') { + e.preventDefault() + navigate(preview.resolvedPath) + return + } + // Trailing filter — try to resolve it to a single folder match in + // the preview listing, mirroring filter-mode Enter. + const filtered = filterEntries(preview.entries, preview.filter) + const action = decideEnterAction(filtered) + if (action.type === 'navigate') { + e.preventDefault() + navigate(joinPath(preview.resolvedPath, action.name)) + } else if (action.type === 'fileHint') { + e.preventDefault() + triggerFileHint() + } else { + e.preventDefault() + } + return + } const action = decideEnterAction(filteredEntries) if (action.type === 'navigate') { e.preventDefault() @@ -196,17 +503,58 @@ export function RemoteFileBrowser({ e.stopPropagation() e.preventDefault() setFilter('') + setPreview(null) + previewGenRef.current++ + // Cancel any pending debounced resolve so it can't fire after + // the user has already dismissed the preview with Escape. + if (debounceTimerRef.current) { + clearTimeout(debounceTimerRef.current) + debounceTimerRef.current = null + } clearFileHint() } else { onCancel() } } + if (e.key === 'Backspace' && filter === '' && !preview) { + // Backspace in an empty input climbs to the parent — only when the + // caret is in empty text, so in-word Backspaces are untouched. + if (resolvedPath !== '/') { + e.preventDefault() + navigateUp() + } + } }, - [filter, filteredEntries, navigateInto, triggerFileHint, clearFileHint, onCancel] + [ + filter, + filteredEntries, + preview, + navigate, + navigateInto, + navigateUp, + resolvedPath, + triggerFileHint, + clearFileHint, + onCancel + ] ) const pathSegments = resolvedPath.split('/').filter(Boolean) + // What the list should render: preview listing (with its own filter and + // error) during path mode, committed listing otherwise. + const isPreviewActive = preview !== null + const showPreviewLoading = isPreviewActive && preview!.loading + const displayEntries = isPreviewActive ? previewFilteredEntries : filteredEntries + const displayEmptyDirCopy = isPreviewActive + ? `${preview!.resolvedPath} is empty` + : 'Empty directory' + + // Disable Select folder while a non-empty path-mode preview is visible so + // the committed directory isn't silently selected while the list shows a + // different preview directory. + const selectDisabled = loading || (isPreviewActive && filter !== '') + return (
{/* Breadcrumb bar */} @@ -261,19 +609,33 @@ export function RemoteFileBrowser({ type="text" autoFocus value={filter} - onChange={(e) => { - clearFileHint() - setFilter(e.target.value) - }} + onChange={(e) => handleInputChange(e.target.value)} + onPaste={handleInputPaste} onKeyDown={handleFilterKeyDown} - placeholder="Type to filter…" + placeholder="Type to filter or enter a path…" + aria-invalid={!!preview?.error} + aria-describedby={preview?.error ? 'remote-file-browser-path-error' : undefined} className={cn( - 'w-full h-7 pl-7 pr-2 text-xs rounded-md bg-background', - 'border border-border focus:outline-none focus:ring-1 focus:ring-ring' + 'w-full h-7 pl-7 pr-7 text-xs rounded-md bg-background', + 'border border-border focus:outline-none focus:ring-1 focus:ring-ring', + preview?.error && 'border-destructive/60 focus:ring-destructive/60' )} /> + {showPreviewLoading && ( + + )}
+ {preview?.error && ( + + )} + {/* File listing */}
@@ -285,29 +647,32 @@ export function RemoteFileBrowser({

{error}

- ) : entries.length === 0 ? ( + ) : isPreviewActive && + preview!.entries.length === 0 && + !preview!.error && + !preview!.loading ? ( +
+

{displayEmptyDirCopy}

+
+ ) : !isPreviewActive && entries.length === 0 ? (

Empty directory

- ) : filteredEntries.length === 0 ? ( - // The directory has contents; the filter just hid them all. Generic - // "Empty directory" copy would mislead here. + ) : displayEntries.length === 0 && !preview?.error ? ( + // Directory has contents; filter hides them all. Distinguishing + // filter emptiness from directory emptiness keeps copy accurate.
-

{`No matches for '${filter}'`}

+

{`No matches for '${ + isPreviewActive ? preview!.filter : filter + }'`}

) : ( - filteredEntries.map((entry) => ( + displayEntries.map((entry) => ( @@ -335,9 +697,7 @@ export function RemoteFileBrowser({
- {/* Footer. Path line is on its own row with `block + truncate` so long - paths can't push the container wider; buttons sit on a separate row - and are free to align right without competing for space. */} + {/* Footer */}

Select folder @@ -361,3 +721,11 @@ export function RemoteFileBrowser({ ) } + +// Returns the portion of `raw` before its final `/`, used to decide whether +// a keystroke only changed the trailing filter (cheap local update) or +// changed a committed segment (requires re-resolving). +function committedPrefix(raw: string): string { + const i = raw.lastIndexOf('/') + return i === -1 ? '' : raw.slice(0, i + 1) +} diff --git a/src/renderer/src/components/sidebar/remote-file-browser-helpers.test.ts b/src/renderer/src/components/sidebar/remote-file-browser-helpers.test.ts index dbcd8570fac..f10a18a5cdf 100644 --- a/src/renderer/src/components/sidebar/remote-file-browser-helpers.test.ts +++ b/src/renderer/src/components/sidebar/remote-file-browser-helpers.test.ts @@ -3,6 +3,10 @@ import { decideEnterAction, decideEscAction, filterEntries, + isPathMode, + parentPath, + parsePathInput, + resolveSegmentStep, type DirEntry } from './remote-file-browser-helpers' @@ -28,13 +32,12 @@ describe('filterEntries', () => { describe('decideEnterAction', () => { it('navigates when filter matches exactly one folder (files alongside do not block)', () => { - const filtered = filterEntries(entries, 'e') // README.md, .env, node_modules + const filtered = filterEntries(entries, 'e') expect(decideEnterAction(filtered)).toEqual({ type: 'navigate', name: 'node_modules' }) }) it('is a no-op when multiple folders match', () => { - const filtered = filterEntries(entries, 's') // src, docs (+ no files with s) - // two folders → cannot disambiguate → noop + const filtered = filterEntries(entries, 's') expect(decideEnterAction(filtered)).toEqual({ type: 'noop' }) }) @@ -57,3 +60,233 @@ describe('decideEscAction', () => { expect(decideEscAction('')).toEqual({ type: 'cancel' }) }) }) + +describe('parentPath', () => { + it('strips last segment', () => { + expect(parentPath('/home/neil/docs')).toBe('/home/neil') + }) + it('stays at root', () => { + expect(parentPath('/')).toBe('/') + }) + it('returns root for single-segment absolute', () => { + expect(parentPath('/home')).toBe('/') + }) +}) + +describe('isPathMode', () => { + it('treats plain names as filter mode', () => { + expect(isPathMode('docs')).toBe(false) + expect(isPathMode('README.md')).toBe(false) + expect(isPathMode('')).toBe(false) + }) + + it('treats any `/` as path mode', () => { + expect(isPathMode('a/b')).toBe(true) + expect(isPathMode('/')).toBe(true) + expect(isPathMode('foo/')).toBe(true) + }) + + it('treats bare base markers as path mode', () => { + expect(isPathMode('~')).toBe(true) + expect(isPathMode('.')).toBe(true) + expect(isPathMode('..')).toBe(true) + }) +}) + +describe('parsePathInput', () => { + it('no slash stays in filter mode', () => { + expect(parsePathInput('docs')).toEqual({ mode: 'filter', filter: 'docs' }) + }) + + it('`..` enters path mode and resolves to parent', () => { + expect(parsePathInput('..')).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: ['..'], + trailingFilter: '' + }) + }) + + it('`../sibling` commits `..` and filters by `sibling`', () => { + expect(parsePathInput('../sibling')).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: ['..'], + trailingFilter: 'sibling' + }) + }) + + it('`Documents/orca` commits `Documents` and filters by `orca`', () => { + expect(parsePathInput('Documents/orca')).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: ['Documents'], + trailingFilter: 'orca' + }) + }) + + it('`Documents/` commits `Documents` with empty filter', () => { + expect(parsePathInput('Documents/')).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: ['Documents'], + trailingFilter: '' + }) + }) + + it('`/var/log` resolves from root', () => { + expect(parsePathInput('/var/log')).toEqual({ + mode: 'path', + base: 'root', + committedSegments: ['var'], + trailingFilter: 'log' + }) + }) + + it('`~/Documents` resolves from home', () => { + expect(parsePathInput('~/Documents')).toEqual({ + mode: 'path', + base: 'home', + committedSegments: [], + trailingFilter: 'Documents' + }) + }) + + it('`~` resolves to home with no committed segments', () => { + expect(parsePathInput('~')).toEqual({ + mode: 'path', + base: 'home', + committedSegments: [], + trailingFilter: '' + }) + }) + + it('`./child` resolves from cwd', () => { + expect(parsePathInput('./child')).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: ['.'], + trailingFilter: 'child' + }) + }) + + it('reports repeated separators as invalid', () => { + const parsed = parsePathInput('foo//bar') + expect(parsed.mode).toBe('path') + if (parsed.mode === 'path') { + expect(parsed.invalid).toMatch(/repeated separators/) + } + }) + + it('preserves spaces inside segments', () => { + expect(parsePathInput('My Folder/sub dir')).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: ['My Folder'], + trailingFilter: 'sub dir' + }) + }) + + it('preserves leading/trailing spaces in the full input', () => { + // Leading space keeps it in filter mode unless a `/` or base marker + // appears; once path mode is triggered, spaces must not be trimmed. + const parsed = parsePathInput(' foo /bar ') + expect(parsed).toEqual({ + mode: 'path', + base: 'cwd', + committedSegments: [' foo '], + trailingFilter: 'bar ' + }) + }) +}) + +describe('resolveSegmentStep', () => { + const listing: DirEntry[] = [ + { name: 'Documents', isDirectory: true }, + { name: 'Downloads', isDirectory: true }, + { name: 'orca-internal', isDirectory: true }, + { name: 'notes.txt', isDirectory: false } + ] + + it('exact directory match descends', () => { + expect(resolveSegmentStep('Documents', '/home/neil', listing)).toEqual({ + type: 'descend', + name: 'Documents' + }) + }) + + it('unique prefix descends', () => { + expect(resolveSegmentStep('orca', '/home/neil', listing)).toEqual({ + type: 'descend', + name: 'orca-internal' + }) + }) + + it('ambiguous prefix errors', () => { + const r = resolveSegmentStep('Do', '/home/neil', listing) + expect(r.type).toBe('error') + if (r.type === 'error') { + expect(r.message).toMatch(/multiple directories/) + } + }) + + it('missing segment errors', () => { + const r = resolveSegmentStep('zzz', '/home/neil', listing) + expect(r.type).toBe('error') + }) + + it('exact file match reports not-a-directory instead of prefix-descending', () => { + // `notes.txt` matches exactly as a file; must not fall through to a + // prefix-match heuristic that picks the first folder starting with "n". + const r = resolveSegmentStep('notes.txt', '/home/neil', listing) + expect(r.type).toBe('error') + if (r.type === 'error') { + expect(r.message).toMatch(/isn't a directory/) + } + }) + + it('`.` stays', () => { + expect(resolveSegmentStep('.', '/home/neil', listing).type).toBe('stay') + }) + + it('`..` stays (parent nav handled by caller)', () => { + expect(resolveSegmentStep('..', '/home/neil', listing).type).toBe('stay') + }) + + it('case-insensitive exact match descends when no case-sensitive match exists', () => { + expect(resolveSegmentStep('documents', '/home/neil', listing)).toEqual({ + type: 'descend', + name: 'Documents' + }) + }) + + it('case-insensitive unique prefix descends', () => { + expect(resolveSegmentStep('down', '/home/neil', listing)).toEqual({ + type: 'descend', + name: 'Downloads' + }) + }) + + it('case-sensitive exact match wins over a case-insensitive peer', () => { + const mixed: DirEntry[] = [ + { name: 'Documents', isDirectory: true }, + { name: 'documents', isDirectory: true } + ] + expect(resolveSegmentStep('documents', '/home/neil', mixed)).toEqual({ + type: 'descend', + name: 'documents' + }) + expect(resolveSegmentStep('Documents', '/home/neil', mixed)).toEqual({ + type: 'descend', + name: 'Documents' + }) + }) + + it('case-insensitive ambiguous prefix errors', () => { + const r = resolveSegmentStep('do', '/home/neil', listing) + expect(r.type).toBe('error') + if (r.type === 'error') { + expect(r.message).toMatch(/multiple directories/) + } + }) +}) diff --git a/src/renderer/src/components/sidebar/remote-file-browser-helpers.ts b/src/renderer/src/components/sidebar/remote-file-browser-helpers.ts index fa93b68c5d1..ac8ef99074b 100644 --- a/src/renderer/src/components/sidebar/remote-file-browser-helpers.ts +++ b/src/renderer/src/components/sidebar/remote-file-browser-helpers.ts @@ -41,3 +41,166 @@ export function decideEscAction(filter: string): EscAction { export function joinPath(resolvedPath: string, name: string): string { return resolvedPath === '/' ? `/${name}` : `${resolvedPath}/${name}` } + +export function parentPath(p: string): string { + if (p === '/' || p === '') { + return '/' + } + const parent = p.replace(/\/[^/]+\/?$/, '') + return parent || '/' +} + +// ---------- Path-aware filter parsing ---------- + +export type ParsedInput = + | { mode: 'filter'; filter: string } + | { + mode: 'path' + // `root` = absolute `/`, `home` = resolved SSH user home, `cwd` = the + // currently committed resolvedPath. + base: 'root' | 'home' | 'cwd' + // Segments to resolve one-by-one from the base. Empty string segments + // never appear here — repeated separators are surfaced via `invalid`. + committedSegments: string[] + // The part after the final separator. Drives the local filter applied + // to the resolved preview directory; empty when input ends with `/`. + trailingFilter: string + // Only set for repeated-separator inputs; other resolution failures are + // reported at resolve time so error messages can name the failing seg. + invalid?: string + } + +// Path mode triggers when the input contains `/` or is one of the three +// base-marker literals (`~`, `.`, `..`). The literal `..` rule is required +// because "contains /" alone would keep bare `..` in filter mode. +export function isPathMode(raw: string): boolean { + if (raw.includes('/')) { + return true + } + return raw === '~' || raw === '.' || raw === '..' +} + +export function parsePathInput(raw: string): ParsedInput { + if (!isPathMode(raw)) { + // Filter mode preserves the raw text; trimming happens inside + // `filterEntries` so leading/trailing spaces don't alter the input shown + // back to the user. + return { mode: 'filter', filter: raw } + } + + // Base-marker literals with no trailing slash. + if (raw === '~') { + return { mode: 'path', base: 'home', committedSegments: [], trailingFilter: '' } + } + if (raw === '.') { + return { mode: 'path', base: 'cwd', committedSegments: [], trailingFilter: '' } + } + if (raw === '..') { + return { mode: 'path', base: 'cwd', committedSegments: ['..'], trailingFilter: '' } + } + + let base: 'root' | 'home' | 'cwd' + let remainder: string + if (raw.startsWith('/')) { + base = 'root' + remainder = raw.slice(1) + } else if (raw.startsWith('~/')) { + base = 'home' + remainder = raw.slice(2) + } else { + base = 'cwd' + remainder = raw + } + + // Don't collapse `//`: the visible input must agree with the path being + // resolved. Report it as invalid and let the caller surface the error. + if (remainder.includes('//')) { + return { + mode: 'path', + base, + committedSegments: [], + trailingFilter: '', + invalid: 'Invalid path: repeated separators' + } + } + + // Reject control characters (including NUL and newlines) in path input. + // These segments are eventually shell-escaped and passed to `cd && ls`; + // embedded newlines would corrupt the line-based `ls -1` parser, and NUL + // bytes cause undefined behavior in the shell / C string boundaries. Single- + // quote shell-escaping protects against injection but not against these + // structural hazards, so we reject at the parse layer. + // eslint-disable-next-line no-control-regex + if (/[\x00-\x1F]/.test(remainder)) { + return { + mode: 'path', + base, + committedSegments: [], + trailingFilter: '', + invalid: 'Invalid path: control characters are not allowed' + } + } + + // `split('/')` leaves an empty string when `remainder` ends with `/`, which + // is the only legal "empty tail" and simply means "no trailing filter". + const parts = remainder === '' ? [''] : remainder.split('/') + const trailingFilter = parts.at(-1) ?? '' + const committedSegments = parts.slice(0, -1) + + return { mode: 'path', base, committedSegments, trailingFilter } +} + +export type SegmentOutcome = + | { type: 'stay' } + | { type: 'descend'; name: string } + | { type: 'error'; message: string } + +// Pure decision step for one committed segment. The caller supplies the +// base path (for error messages) and that base's listing. +export function resolveSegmentStep( + segment: string, + basePath: string, + baseEntries: DirEntry[] +): SegmentOutcome { + if (segment === '.') { + return { type: 'stay' } + } + if (segment === '..') { + return { type: 'stay' } // caller turns this into parent navigation + } + // Exact (case-sensitive) match wins. When both `Documents` and `documents` + // exist on a case-sensitive POSIX filesystem, the user's literal spelling + // must be authoritative. + const exact = baseEntries.find((e) => e.name === segment) + if (exact) { + if (exact.isDirectory) { + return { type: 'descend', name: exact.name } + } + // Stop resolution: prefix-matching to a similarly-named folder here would + // silently bypass a real file the user pointed at. + return { type: 'error', message: `${segment} isn't a directory in ${basePath}` } + } + // Fall back to case-insensitive matching so segment resolution agrees with + // the case-insensitive filter input. Without this, typing `documents/` + // errors while typing `documents` finds `Documents` via the filter — the + // two modes must not disagree. Remote listings can still be case-sensitive; + // we only accept CI matches when the case-sensitive match is absent. + const segLower = segment.toLowerCase() + const ciExact = baseEntries.find((e) => e.name.toLowerCase() === segLower) + if (ciExact) { + if (ciExact.isDirectory) { + return { type: 'descend', name: ciExact.name } + } + return { type: 'error', message: `${segment} isn't a directory in ${basePath}` } + } + const dirMatches = baseEntries.filter( + (e) => e.isDirectory && e.name.toLowerCase().startsWith(segLower) + ) + if (dirMatches.length === 1) { + return { type: 'descend', name: dirMatches[0].name } + } + if (dirMatches.length > 1) { + return { type: 'error', message: `${segment} matches multiple directories in ${basePath}` } + } + return { type: 'error', message: `${segment} isn't a directory in ${basePath}` } +}