Right-click file operations, and an icon in the taskbar (#12)

* desktop: right-click file operations in both panes

Refresh, New folder, New file, Rename and Delete, on a context menu, in
the local pane and the remote one alike — the FileZilla operations the
panes looked like they already had. F2 renames and Del deletes, because a
file manager that answers only the mouse is half a file manager.

The remote half already had mkdir/rename/delete over SFTP and nothing
called them; the local half had no mutations at all. Both sides now go
through one bridge where the connection id is what selects local or
remote, so a caller cannot act on the wrong pane by picking the wrong
method name.

Every mutation takes a directory and a bare entry name and joins them in
the main process. EntryNameSchema rejects a separator, `..`, a NUL and
surrounding space, so "New folder" in a listing cannot write outside the
folder being listed — on the remote side, anywhere the SSH user can
reach. Delete re-stats its target and refuses when what is on disk
disagrees with what the renderer claimed, so a mislabelled request cannot
turn one unlink into a recursive delete; a symlink is unlinked, never
followed.

Two things SFTP does not give you: creating a file (open 'wx', which
fails rather than truncating an existing one) and removing a populated
directory (rmdir only unlinks an empty one, so removeRecursive walks it
depth first).

Verified in a headless render of the real export under the app's real
CSP, both themes: the menu opens on a row with all five items live, the
row it opened on becomes the selection, and the New folder dialog focuses
its field with its footer inside the panel. That render caught the first
draft's bug — the container's handler ran after the row's as the event
bubbled and cleared the target, leaving Rename and Delete greyed out on
every row.

20 new schema tests cover the traversal cases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd

* desktop: give the window an icon, and a WM_CLASS a taskbar can match

The window was created with no `icon`, so it carried no _NET_WM_ICON and
a Linux taskbar had nothing to draw but a placeholder. That is invisible
when the app is started from its .desktop file, because the launcher
supplies the icon — and it is exactly what you see when the app is
started any other way, which is how anyone running the AppImage or a
checkout starts it.

The window now takes resources/icon.png: from `process.resourcesPath`
when packaged, from the repo when run out of a checkout, and undefined
rather than a path to nothing, since BrowserWindow given a missing icon
logs nothing and shows no icon.

The second half is grouping. A taskbar ties a window to its launcher by
matching WM_CLASS against the .desktop file's StartupWMClass, and neither
desktop entry had one. Chromium derives WM_CLASS from the executable
name, which differs between the deb, the AppImage and a dev run, so the
app now pins it to `DiskPush` before the window is created and both
entries — the installer's and electron-builder's — declare that same
value. A test reads both files and fails if either drifts from the
constant, because a rename that touched one of them would un-group the
window with nothing else failing.

Verified: the checkout icon path resolves against the real dist-electron
layout, and the 5 new tests cover candidate ordering, the dev fallback,
the missing-icon case and both desktop entries. Not verified here: the
WM_CLASS the window actually reports. This box has no X server and no
xprop, so the pinning is argued from Chromium's behaviour rather than
measured. Its failure mode is the icon we now set being used without
grouping, which is no worse than today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: ralyodio

apps/desktop/electron/main/icon-path.test.ts

@@ -0,0 +1,44 @@
+import { readFileSync } from 'node:fs'
+import { join } from 'node:path'
+import { describe, expect, it } from 'vitest'
+import { resolveIconPath, WM_CLASS } from './icon-path.js'
+
+const PACKAGED = '/opt/DiskPush/resources/icon.png'
+const CHECKOUT = '/home/you/diskpush/apps/desktop/resources/icon.png'
+
+describe('resolveIconPath', () => {
+  it('prefers the packaged copy when it is there', () => {
+    expect(resolveIconPath([PACKAGED, CHECKOUT], () => true)).toBe(PACKAGED)
+  })
+
+  it('falls back to the checkout, which is the only copy in a dev run', () => {
+    expect(resolveIconPath([PACKAGED, CHECKOUT], (path) => path === CHECKOUT)).toBe(CHECKOUT)
+  })
+
+  it('returns undefined rather than a path to nothing', () => {
+    // BrowserWindow given a missing icon path logs nothing and shows no icon,
+    // so a guess is indistinguishable from the bug this is fixing.
+    expect(resolveIconPath([PACKAGED, CHECKOUT], () => false)).toBeUndefined()
+  })
+})
+
+describe('StartupWMClass', () => {
+  /**
+   * A taskbar matches a window to its launcher by comparing the window's
+   * WM_CLASS with the .desktop file's StartupWMClass. The app pins WM_CLASS to
+   * WM_CLASS above; these two files are the other half of that agreement, and
+   * a rename that touched only one of them would silently un-group the window
+   * again — the exact symptom, with nothing failing.
+   */
+  const root = join(import.meta.dirname, '..', '..', '..', '..')
+
+  it('is what the installer writes into its desktop entry', () => {
+    const installer = readFileSync(join(root, 'scripts', 'install.sh'), 'utf8')
+    expect(installer).toContain(`StartupWMClass=${WM_CLASS}`)
+  })
+
+  it('is what electron-builder writes into the packaged entry', () => {
+    const manifest = JSON.parse(readFileSync(join(root, 'apps', 'desktop', 'package.json'), 'utf8'))
+    expect(manifest.build.linux.desktop.entry.StartupWMClass).toBe(WM_CLASS)
+  })
+})

apps/desktop/electron/main/icon-path.ts

@@ -0,0 +1,32 @@
+/**
+ * Where the window icon lives, which differs between a packaged app and a
+ * checkout.
+ *
+ * Packaged, electron-builder copies resources/icon.png to `extraResources`, so
+ * it sits beside the app under `process.resourcesPath`. Run from a checkout,
+ * `process.resourcesPath` is Electron's own resources directory and holds
+ * nothing of ours, so the repo copy is the fallback.
+ *
+ * Kept apart from index.ts, and given its own `exists`, so the ordering can be
+ * tested without Electron and without touching a disk.
+ */
+export function resolveIconPath(
+  candidates: readonly string[],
+  exists: (path: string) => boolean,
+): string | undefined {
+  // Undefined rather than a guess: BrowserWindow given a path to nothing logs
+  // no error and shows no icon, which is indistinguishable from not asking.
+  return candidates.find((candidate) => exists(candidate))
+}
+
+/**
+ * The WM_CLASS the window reports, pinned rather than inherited.
+ *
+ * A Linux taskbar matches a window to its launcher by comparing WM_CLASS with
+ * the .desktop file's StartupWMClass. Left alone, Chromium derives WM_CLASS
+ * from the executable name, so it changes with `executableName` and differs
+ * between the deb, the AppImage and a dev run — and a StartupWMClass written
+ * to match one of those is wrong for the others. Naming it here means the
+ * desktop entries can all state the same value.
+ */
+export const WM_CLASS = 'DiskPush'

apps/desktop/electron/main/index.ts

@@ -1,9 +1,10 @@
-import { readFileSync } from 'node:fs'
+import { existsSync, readFileSync } from 'node:fs'
 import { readFile } from 'node:fs/promises'
 import { join, normalize } from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { app, BrowserWindow, protocol, shell } from 'electron'
 import { contentTypeFor, resolveBundlePath } from './bundle-path.js'
+import { resolveIconPath, WM_CLASS } from './icon-path.js'
 import { contentSecurityPolicy, inlineScriptHashes } from './csp.js'
 import { registerIpc } from './ipc.js'
 import { checkForUpdates } from './services/updater.js'
@@ -23,6 +24,22 @@ const here = join(fileURLToPath(import.meta.url), '..')
 const isDev = !app.isPackaged && process.env.DISKPUSH_DEV === '1'
 const DEV_URL = 'http://localhost:3210'
 
+/**
+ * Pinned before the app is ready, because Chromium reads it when it creates
+ * the window: a taskbar matches WM_CLASS against a .desktop file's
+ * StartupWMClass, and left to itself Chromium derives WM_CLASS from the
+ * executable name — different for the deb, the AppImage and a dev run.
+ */
+app.commandLine.appendSwitch('class', WM_CLASS)
+
+/** The window icon, which is also what a taskbar draws for the running app. */
+function iconPath(): string | undefined {
+  return resolveIconPath(
+    [join(process.resourcesPath, 'icon.png'), join(here, '..', '..', 'resources', 'icon.png')],
+    existsSync,
+  )
+}
+
 /**
  * The exported renderer is served over a real scheme rather than loaded from
  * file://.
@@ -88,13 +105,18 @@ function serveBundle(): void {
 }
 
 function createWindow(): BrowserWindow {
+  const icon = iconPath()
   const window = new BrowserWindow({
     width: 1360,
     height: 860,
     minWidth: 960,
     minHeight: 600,
     backgroundColor: '#0a0c10',
     title: 'DiskPush',
+    // Without this the window carries no _NET_WM_ICON and the taskbar draws a
+    // placeholder, which is what an app launched directly rather than from its
+    // .desktop file always looked like.
+    ...(icon ? { icon } : {}),
     webPreferences: {
       preload: join(here, '..', 'preload', 'index.cjs'),
       // The renderer gets no Node, no remote module, and its own sandbox. It

apps/desktop/electron/main/ipc.ts

@@ -1,18 +1,19 @@
-import { readFile } from 'node:fs/promises'
-import { readdir, stat } from 'node:fs/promises'
+import { lstat, mkdir, open, readdir, readFile, rename, rm, stat, unlink } from 'node:fs/promises'
 import { homedir } from 'node:os'
-import { isAbsolute, join, resolve } from 'node:path'
+import { isAbsolute, join, posix, resolve } from 'node:path'
 import { ipcMain, shell, type IpcMainInvokeEvent } from 'electron'
-import { probeConnection, parseSshConfig, sshConfigConnections } from '@diskpush/ssh-core'
+import { probeConnection, parseSshConfig, sshConfigConnections, type SftpBrowser } from '@diskpush/ssh-core'
 import { z } from 'zod'
 import {
   ConnectionInputSchema,
+  CreateEntryRequestSchema,
+  DeleteEntryRequestSchema,
   ExternalUrlSchema,
   IPC,
   JobIdSchema,
   PathSchema,
   RemotePathRequestSchema,
-  RenameRequestSchema,
+  RenameEntryRequestSchema,
   TransferRequestSchema,
   type IpcResult,
 } from '../shared/contract.js'
@@ -51,6 +52,32 @@ function resolveLocalPath(input: string): string {
   return isAbsolute(expanded) ? expanded : resolve(expanded)
 }
 
+/** Whether a path exists, without making the caller catch ENOENT. */
+async function exists(path: string): Promise<boolean> {
+  try {
+    await lstat(path)
+    return true
+  } catch {
+    return false
+  }
+}
+
+/**
+ * Runs `fn` against an SFTP browser for `connectionId` and always closes it.
+ *
+ * Every remote mutation had its own copy of this, and a `finally` that is
+ * written six times is a `finally` that eventually is not.
+ */
+async function withBrowser<T>(connectionId: string | undefined, fn: (browser: SftpBrowser) => Promise<T>): Promise<T> {
+  if (!connectionId) throw new Error('That operation needs a server.')
+  const browser = await browserFor(await requireConnection(connectionId))
+  try {
+    return await fn(browser)
+  } finally {
+    browser.close()
+  }
+}
+
 export function registerIpc(): void {
   // --- connections ---------------------------------------------------------
 
@@ -164,42 +191,87 @@ export function registerIpc(): void {
     }
   })
 
-  handle(IPC.fsMkdirRemote, RemotePathRequestSchema, async ({ connectionId, path }) => {
-    const connection = await requireConnection(connectionId)
-    const browser = await browserFor(connection)
-    try {
-      await browser.mkdir(path)
-      return true
-    } finally {
-      browser.close()
-    }
+  /**
+   * The mutating operations, local and remote.
+   *
+   * Each takes a directory and a bare entry name and joins them here, so the
+   * renderer names a thing inside the folder it is showing rather than handing
+   * the main process a path to act on. `resolveLocalPath` still expands `~`,
+   * but it is applied to the directory only.
+   */
+  handle(IPC.fsMkdirLocal, CreateEntryRequestSchema, async ({ directory, name }) => {
+    await mkdir(join(resolveLocalPath(directory), name))
+    return true
   })
 
-  handle(IPC.fsRenameRemote, RenameRequestSchema, async ({ connectionId, from, to }) => {
-    const connection = await requireConnection(connectionId)
-    const browser = await browserFor(connection)
-    try {
-      await browser.rename(from, to)
+  handle(IPC.fsCreateFileLocal, CreateEntryRequestSchema, async ({ directory, name }) => {
+    // `wx` fails when the file exists rather than truncating it.
+    const handle = await open(join(resolveLocalPath(directory), name), 'wx')
+    await handle.close()
+    return true
+  })
+
+  handle(IPC.fsRenameLocal, RenameEntryRequestSchema, async ({ directory, from, to }) => {
+    const root = resolveLocalPath(directory)
+    const target = join(root, to)
+    // Renaming onto an existing name silently destroys it, so refuse. There is
+    // an unavoidable race here; it narrows a footgun rather than closing it.
+    if (await exists(target)) throw new Error(`“${to}” already exists here.`)
+    await rename(join(root, from), target)
+    return true
+  })
+
+  handle(IPC.fsDeleteLocal, DeleteEntryRequestSchema, async ({ directory, name, isDirectory }) => {
+    const target = join(resolveLocalPath(directory), name)
+    const stats = await lstat(target)
+    // A symlink to a directory reports as a directory to the caller; deleting
+    // it must still unlink the link rather than recurse into what it points at.
+    if (stats.isSymbolicLink()) {
+      await unlink(target)
       return true
-    } finally {
-      browser.close()
     }
+    if (stats.isDirectory() !== isDirectory) throw new Error('That item changed on disk; refresh and try again.')
+    if (isDirectory) await rm(target, { recursive: true })
+    else await unlink(target)
+    return true
   })
 
-  handle(
-    IPC.fsDeleteRemote,
-    z.object({ connectionId: z.string().min(1), path: PathSchema, isDirectory: z.boolean() }),
-    async ({ connectionId, path, isDirectory }) => {
-      const connection = await requireConnection(connectionId)
-      const browser = await browserFor(connection)
-      try {
-        if (isDirectory) await browser.rmdir(path)
-        else await browser.unlink(path)
+  handle(IPC.fsMkdirRemote, CreateEntryRequestSchema, async ({ connectionId, directory, name }) =>
+    withBrowser(connectionId, async (browser) => {
+      await browser.mkdir(posix.join(directory, name))
+      return true
+    }),
+  )
+
+  handle(IPC.fsCreateFileRemote, CreateEntryRequestSchema, async ({ connectionId, directory, name }) =>
+    withBrowser(connectionId, async (browser) => {
+      await browser.createFile(posix.join(directory, name))
+      return true
+    }),
+  )
+
+  handle(IPC.fsRenameRemote, RenameEntryRequestSchema, async ({ connectionId, directory, from, to }) =>
+    withBrowser(connectionId, async (browser) => {
+      await browser.rename(posix.join(directory, from), posix.join(directory, to))
+      return true
+    }),
+  )
+
+  handle(IPC.fsDeleteRemote, DeleteEntryRequestSchema, async ({ connectionId, directory, name, isDirectory }) =>
+    withBrowser(connectionId, async (browser) => {
+      const target = posix.join(directory, name)
+      const stats = await browser.stat(target)
+      if (stats.type === 'symlink') {
+        await browser.unlink(target)
         return true
-      } finally {
-        browser.close()
       }
-    },
+      if ((stats.type === 'directory') !== isDirectory) {
+        throw new Error('That item changed on the server; refresh and try again.')
+      }
+      if (isDirectory) await browser.removeRecursive(target)
+      else await browser.unlink(target)
+      return true
+    }),
   )
 
   // --- transfers -----------------------------------------------------------

apps/desktop/electron/preload/index.ts

@@ -24,11 +24,20 @@ const api = {
     homeLocal: () => call<string>(IPC.fsHomeLocal),
     listLocal: (path: string) => call(IPC.fsListLocal, { path }),
     listRemote: (connectionId: string, path: string) => call(IPC.fsListRemote, { connectionId, path }),
-    mkdirRemote: (connectionId: string, path: string) => call(IPC.fsMkdirRemote, { connectionId, path }),
-    renameRemote: (connectionId: string, from: string, to: string) =>
-      call(IPC.fsRenameRemote, { connectionId, from, to }),
-    deleteRemote: (connectionId: string, path: string, isDirectory: boolean) =>
-      call(IPC.fsDeleteRemote, { connectionId, path, isDirectory }),
+    // Mutations name an entry inside a directory; the main process joins them.
+    mkdir: (directory: string, name: string, connectionId?: string) =>
+      call<boolean>(connectionId ? IPC.fsMkdirRemote : IPC.fsMkdirLocal, { connectionId, directory, name }),
+    createFile: (directory: string, name: string, connectionId?: string) =>
+      call<boolean>(connectionId ? IPC.fsCreateFileRemote : IPC.fsCreateFileLocal, { connectionId, directory, name }),
+    rename: (directory: string, from: string, to: string, connectionId?: string) =>
+      call<boolean>(connectionId ? IPC.fsRenameRemote : IPC.fsRenameLocal, { connectionId, directory, from, to }),
+    remove: (directory: string, name: string, isDirectory: boolean, connectionId?: string) =>
+      call<boolean>(connectionId ? IPC.fsDeleteRemote : IPC.fsDeleteLocal, {
+        connectionId,
+        directory,
+        name,
+        isDirectory,
+      }),
   },
   transfers: {
     preview: (request: unknown) => call(IPC.transfersPreview, request),