Open symlinked directories, and re-read ssh_config when asked

apps/desktop/src/app/page.tsx

@@ -346,6 +346,7 @@ export default function Workspace() {
           state={left}
           saved={saved}
           sshConfig={sshConfig}
+          onRefreshHosts={refreshConnections}
           active={active === 'left'}
           onFocus={() => setActive('left')}
           onChange={(patch) => setLeft((current) => ({ ...current, ...patch }))}
@@ -371,6 +372,7 @@ export default function Workspace() {
           state={right}
           saved={saved}
           sshConfig={sshConfig}
+          onRefreshHosts={refreshConnections}
           active={active === 'right'}
           onFocus={() => setActive('right')}
           onChange={(patch) => setRight((current) => ({ ...current, ...patch }))}

apps/desktop/src/components/endpoint-select.tsx

@@ -37,12 +37,15 @@ export function EndpointSelect({
   sshConfig,
   onChange,
   onAddServer,
+  onOpen,
 }: {
   value: PaneEndpoint
   saved: readonly Connection[]
   sshConfig: readonly Connection[]
   onChange: (endpoint: PaneEndpoint) => void
   onAddServer: () => void
+  /** Re-reads the host lists, so an edited ~/.ssh/config shows without a restart. */
+  onOpen?: () => void
 }) {
   const current = value.kind === 'local' ? LOCAL : value.connectionId
   const selectedLabel =
@@ -53,6 +56,12 @@ export function EndpointSelect({
   return (
     <Select
       value={current}
+      // ~/.ssh/config was read once at startup, so a host removed from the file
+      // stayed in this list for the life of the window — and a host added to it
+      // never appeared at all.
+      onOpenChange={(open) => {
+        if (open) onOpen?.()
+      }}
       onValueChange={(next) => {
         // Base UI hands back `null` when a select is cleared, so this cannot
         // assume a string and build a connection id out of it.

apps/desktop/src/components/pane.tsx

@@ -18,6 +18,7 @@ import {
   Trash2,
 } from 'lucide-react'
 import { api, unwrap, type Connection, type FileEntry } from '@/lib/api'
+import { isNavigable } from '@/lib/entries'
 import { formatBytes, formatDate, formatMode, joinPath, parentPath } from '@/lib/format'
 import { EndpointSelect, type PaneEndpoint } from '@/components/endpoint-select'
 import { DeleteDialog, NameDialog } from '@/components/entry-dialogs'
@@ -208,6 +209,7 @@ export function Pane({
   onNavigate,
   onEndpointChange,
   onAddServer,
+  onRefreshHosts,
 }: {
   role: 'Source' | 'Destination'
   state: PaneState
@@ -219,6 +221,7 @@ export function Pane({
   onNavigate: (path: string) => void
   onEndpointChange: (endpoint: PaneEndpoint) => void
   onAddServer: () => void
+  onRefreshHosts?: () => void
 }) {
   const [filter, setFilter] = useState('')
   const [showHidden, setShowHidden] = useState(false)
@@ -247,7 +250,8 @@ export function Pane({
         .filter((entry) => filter === '' || entry.name.toLowerCase().includes(filter.toLowerCase()))
         .sort((a, b) => {
           // Directories first, then by name: the order every file manager uses.
-          if ((a.type === 'directory') !== (b.type === 'directory')) return a.type === 'directory' ? -1 : 1
+          // A link to a directory sorts as one, because that is what it opens as.
+          if (isNavigable(a) !== isNavigable(b)) return isNavigable(a) ? -1 : 1
           return a.name.localeCompare(b.name)
         }),
     [state.entries, filter, showHidden],
@@ -287,7 +291,10 @@ export function Pane({
 
   const open = useCallback(
     (entry: FileEntry) => {
-      if (entry.type === 'directory') onNavigate(joinPath(state.path, entry.name))
+      // The link's own path is what we navigate to: the server resolves it when
+      // it lists, so there is no need to send the target and no risk of leaving
+      // the path the user can see in the breadcrumbs.
+      if (isNavigable(entry)) onNavigate(joinPath(state.path, entry.name))
     },
     [onNavigate, state.path],
   )
@@ -406,6 +413,7 @@ export function Pane({
           sshConfig={sshConfig}
           onChange={onEndpointChange}
           onAddServer={onAddServer}
+          onOpen={onRefreshHosts}
         />
         <span
           className={cn(
@@ -548,21 +556,30 @@ export function Pane({
                     {entry.type === 'directory' ? (
                       <Folder className="size-[15px] shrink-0 fill-primary/20 text-primary" />
                     ) : entry.type === 'symlink' ? (
-                      <Link2 className="size-[15px] shrink-0 text-cyan" />
+                      // A link that opens as a folder is drawn as one, tinted to
+                      // keep it distinguishable from a real directory.
+                      isNavigable(entry) ? (
+                        <Folder className="size-[15px] shrink-0 fill-cyan/20 text-cyan" />
+                      ) : (
+                        <Link2 className="size-[15px] shrink-0 text-cyan" />
+                      )
                     ) : (
                       <FileText className="size-[15px] shrink-0 text-faint" />
                     )}
-                    <span className={cn('truncate', isSelected ? 'font-medium text-foreground' : 'text-dim')}>
+                    <span
+                      className={cn('truncate', isSelected ? 'font-medium text-foreground' : 'text-dim')}
+                      title={entry.linkTarget ? `${entry.name} → ${entry.linkTarget}` : undefined}
+                    >
                       {entry.name}
                     </span>
                   </span>
                   <span
                     className={cn(
                       'numeric text-right text-[11.5px]',
-                      entry.type === 'directory' ? 'text-faint' : 'text-muted-foreground',
+                      isNavigable(entry) ? 'text-faint' : 'text-muted-foreground',
                     )}
                   >
-                    {entry.type === 'directory' ? '—' : formatBytes(entry.size)}
+                    {isNavigable(entry) ? '—' : formatBytes(entry.size)}
                   </span>
                   <span className="numeric text-right text-[11.5px] text-muted-foreground" title={formatMode(entry.mode)}>
                     {formatDate(entry.modifiedAt)}

apps/desktop/src/lib/api.ts

@@ -16,6 +16,10 @@ export type FileEntry = {
   size: number
   modifiedAt: string
   mode: number
+  /** Where a symlink points, when the server could tell us. */
+  linkTarget?: string
+  /** What it points at. Absent on a broken link, which is why it is optional. */
+  targetType?: 'file' | 'directory' | 'symlink' | 'other'
 }
 
 export type Connection = {

apps/desktop/src/lib/entries.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from 'vitest'
+import { isNavigable } from './entries.js'
+
+describe('isNavigable', () => {
+  /**
+   * The bug this exists to prevent: `~/data -> /mnt/vdb` on a server was a row
+   * that could not be opened. SFTP's readdir types a link the way lstat does,
+   * and the pane only walked into rows typed `directory`, so every
+   * double-click on it did nothing at all.
+   */
+  it('opens a link that points at a directory', () => {
+    expect(isNavigable({ type: 'symlink', targetType: 'directory' })).toBe(true)
+  })
+
+  it('does not open a link that points at a file', () => {
+    expect(isNavigable({ type: 'symlink', targetType: 'file' })).toBe(false)
+  })
+
+  it('does not open a link whose target could not be resolved', () => {
+    // A broken link, or one pointing where this user cannot stat: there is
+    // nothing to walk into, and guessing yes turns a click into an error.
+    expect(isNavigable({ type: 'symlink', targetType: undefined })).toBe(false)
+  })
+
+  it('opens a real directory', () => {
+    expect(isNavigable({ type: 'directory' })).toBe(true)
+  })
+
+  it('does not open a file, whatever a stale targetType says', () => {
+    expect(isNavigable({ type: 'file', targetType: 'directory' })).toBe(false)
+  })
+})

apps/desktop/src/lib/entries.ts

@@ -0,0 +1,14 @@
+import type { FileEntry } from '@/lib/api'
+
+/**
+ * Whether opening this row should walk into it.
+ *
+ * A remote listing reports link types the way lstat does, so `~/data -> /mnt/vdb`
+ * arrives as `symlink` and used to be a row that swallowed every double-click.
+ * What matters is what is behind the link, not that it is one — and an
+ * unresolved target (a broken link, or one this user cannot stat) is not
+ * something to walk into.
+ */
+export function isNavigable(entry: Pick<FileEntry, 'type' | 'targetType'>): boolean {
+  return entry.type === 'directory' || (entry.type === 'symlink' && entry.targetType === 'directory')
+}

packages/ssh-core/src/browser.test.ts

@@ -0,0 +1,91 @@
+import { describe, expect, it } from 'vitest'
+import { SftpBrowser } from './browser.js'
+
+const S_IFDIR = 0o040000
+const S_IFREG = 0o100000
+const S_IFLNK = 0o120000
+
+type Row = { filename: string; mode: number }
+
+/**
+ * The slice of ssh2's SFTPWrapper the browser actually uses.
+ *
+ * `readdir` reports a link the way lstat does; `stat` follows it. Keeping both
+ * in a stub is the whole point — that difference is the bug being tested.
+ */
+function fakeSftp(rows: Row[], targets: Record<string, number | 'error'>) {
+  return {
+    readdir(_directory: string, callback: (error: Error | null, entries: unknown[]) => void) {
+      callback(
+        null,
+        rows.map((row) => ({
+          filename: row.filename,
+          attrs: { mode: row.mode, size: 4, mtime: 0, uid: 1000, gid: 1000 },
+        })),
+      )
+    },
+    stat(path: string, callback: (error: Error | null, stats?: unknown) => void) {
+      const target = targets[path]
+      if (target === undefined || target === 'error') {
+        callback(new Error('No such file'))
+        return
+      }
+      callback(null, { mode: target, size: 4, mtime: 0, uid: 1000, gid: 1000 })
+    },
+    readlink(path: string, callback: (error: Error | null, target?: string) => void) {
+      const known: Record<string, string> = { '/home/you/data': '/mnt/vdb', '/home/you/gone': '/mnt/missing' }
+      const target = known[path]
+      target ? callback(null, target) : callback(new Error('not a link'))
+    },
+  } as never
+}
+
+describe('SftpBrowser.list', () => {
+  it('resolves what a symlink points at, so a link to a directory can be opened', async () => {
+    const browser = new SftpBrowser(
+      fakeSftp(
+        [
+          { filename: 'notes.md', mode: S_IFREG | 0o644 },
+          { filename: 'data', mode: S_IFLNK | 0o777 },
+        ],
+        { '/home/you/data': S_IFDIR | 0o755 },
+      ),
+    )
+
+    const entries = await browser.list('/home/you')
+    const data = entries.find((entry) => entry.name === 'data')
+    expect(data?.type).toBe('symlink')
+    expect(data?.targetType).toBe('directory')
+    expect(data?.linkTarget).toBe('/mnt/vdb')
+  })
+
+  it('marks a link to a file as a file target, which stays unopenable', async () => {
+    const browser = new SftpBrowser(
+      fakeSftp([{ filename: 'conf', mode: S_IFLNK | 0o777 }], { '/home/you/conf': S_IFREG | 0o644 }),
+    )
+    expect((await browser.list('/home/you'))[0]?.targetType).toBe('file')
+  })
+
+  it('leaves targetType absent on a broken link rather than failing the listing', async () => {
+    // A directory containing one dead link must still list; the row is simply
+    // one that cannot be walked into.
+    const browser = new SftpBrowser(fakeSftp([{ filename: 'gone', mode: S_IFLNK | 0o777 }], {}))
+    const entries = await browser.list('/home/you')
+    expect(entries).toHaveLength(1)
+    expect(entries[0]?.targetType).toBeUndefined()
+  })
+
+  it('does not stat anything for a listing with no links', async () => {
+    const browser = new SftpBrowser(
+      fakeSftp(
+        [
+          { filename: 'a', mode: S_IFREG | 0o644 },
+          { filename: 'b', mode: S_IFDIR | 0o755 },
+        ],
+        {},
+      ),
+    )
+    const entries = await browser.list('/home/you')
+    expect(entries.map((entry) => entry.type)).toEqual(['file', 'directory'])
+  })
+})

packages/ssh-core/src/browser.ts

@@ -22,6 +22,15 @@ export type RemoteEntry = {
   gid: number
   /** Present for symlinks once resolved. */
   linkTarget?: string
+  /**
+   * What a symlink points at, when it could be resolved.
+   *
+   * SFTP's readdir reports link types the way lstat does, so a link to a
+   * directory arrives as `symlink` and nothing downstream can tell whether it
+   * can be opened. Undefined means the target could not be stat'd — a broken
+   * link, or one pointing somewhere this user cannot read.
+   */
+  targetType?: RemoteEntry['type']
 }
 
 const S_IFMT = 0o170000
@@ -54,7 +63,33 @@ export class SftpBrowser {
     return new SftpBrowser(await session.sftp())
   }
 
-  list(directory: string): Promise<RemoteEntry[]> {
+  async list(directory: string): Promise<RemoteEntry[]> {
+    const entries = await this.readdir(directory)
+
+    // One follow-stat per symlink, and only per symlink: a listing is mostly
+    // ordinary files, and without this a link to a directory is a row nothing
+    // can open.
+    await Promise.all(
+      entries
+        .filter((entry) => entry.type === 'symlink')
+        .map(async (entry) => {
+          try {
+            const [target, resolved] = await Promise.all([
+              this.readlink(entry.path).catch(() => undefined),
+              this.statFollowing(entry.path),
+            ])
+            entry.targetType = resolved
+            if (target) entry.linkTarget = target
+          } catch {
+            // A broken link is still a row worth showing; it just cannot be
+            // opened, which is exactly what an absent targetType means.
+          }
+        }),
+    )
+    return entries
+  }
+
+  private readdir(directory: string): Promise<RemoteEntry[]> {
     return new Promise((resolve, reject) => {
       this.sftp.readdir(directory, (error, entries: FileEntry[]) => {
         if (error) {
@@ -77,6 +112,13 @@ export class SftpBrowser {
     })
   }
 
+  /** `stat`, which follows a link, where `stat()` above uses `lstat`, which does not. */
+  private statFollowing(path: string): Promise<RemoteEntry['type'] | undefined> {
+    return new Promise((resolve) => {
+      this.sftp.stat(path, (error, stats) => resolve(error ? undefined : entryType(stats.mode)))
+    })
+  }
+
   stat(path: string): Promise<RemoteEntry> {
     return new Promise((resolve, reject) => {
       this.sftp.lstat(path, (error, stats) => {