Skip to content

Commit 62bbc09

Browse files
committed
Transfer: Unify entry paths behind one guard + source resolver
- New `pane/transfer-entry.ts` is the shared seam every transfer entry path runs through, so F5/F6, drag-and-drop, and clipboard paste prepare a transfer identically: - `checkTransferDestinationGuard(destVolumeId, volumes)`: the shared destination guard chain (search-results not-a-folder toast, then read-only "Read-only device" alert). Copy strings unchanged — they're the E2E-asserted contract. An unknown destination id is allowed through (can't prove read-only; the backend still rejects a genuinely read-only write). - `resolveSourceVolumeId(paths, volumes, resolvePathVolume)`: resolves the REAL source volume for dropped/pasted paths — frontend `findVolumeIdForPath` longest-prefix (handles MTP-shaped paths), backend `resolve_path_volume` fallback for the common parent, honest-unknown `root` default. Never ships a knowingly-wrong id: sources spanning volumes or a failed resolve return `root`. - F5/F6 (`file-operation-commands.ts`): the inline read-only / search-results guards now call `checkTransferDestinationGuard`. Behavior-preserving; the 37 existing specs stay green. - Drag-and-drop (`drag-drop-controller.svelte.ts`): runs the shared guard before opening the dialog (field bug 2: dropping onto a read-only volume now shows the same "Read-only device" alert F5 shows, instead of silently opening a copy dialog), and resolves the real source volume id instead of the old `sourceVolumeId = destVolumeId` placeholder (field bug 4: an MTP→local / local→MTP drop now stats the right volume, so the dialog's byte/file/dir counters fill instead of reading 0). `buildTransferPropsFromDroppedPaths` takes the resolved `sourceVolumeId` explicitly. - Clipboard paste (`clipboard-operations.ts`): runs the shared guard for a read-only destination. The MTP-specific "Use F5…" refusal stays separate and ahead of it (it points the user at the F5/F6 flow paste lacks). - Tests (red-first for the bug-targeting ones): `transfer-entry.test.ts` (guard + resolver matrix: local / MTP-shaped / spanning / BE-fallback / honest-unknown); drag-drop component tests for the read-only guard (bug 2) and the resolved cross-volume source id (bug 4); a `TransferDialog` test pinning that the dialog forwards the source volume id to `startScanPreview` (the bug-4 seam); a read-only paste guard test. Existing F5/F6 + drag + clipboard + dialog specs unchanged. - Docs: updated the transfer FE, pane, and drag `CLAUDE.md`s for the new shared-seam architecture.
1 parent f4a8b1c commit 62bbc09

13 files changed

Lines changed: 631 additions & 64 deletions

apps/desktop/src/lib/file-explorer/drag/CLAUDE.md

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,27 @@ Key files:
5858
- `drag-position.ts`: Corrects Tauri coords for docked DevTools (dev-only, zero overhead in prod)
5959
- Integration in `DualPaneExplorer.svelte`
6060

61-
Drop count split: on drop, `pane/drag-drop-controller.svelte.ts::handleFileDrop` fetches each dropped path's top-level
62-
kind (file vs. folder) in one batched `stat_paths_kinds` IPC before opening the confirmation dialog, so both the dialog
63-
and the completion toast report the real "N files and M folders" split. The stat runs under the backend read timeout (2
64-
s) and falls back to all-unknown on a hung mount, so it never blocks the drop. The split is all-or-nothing: if ANY
65-
path's kind is unknown (a virtual MTP/SMB path that landed on the pasteboard, a vanished entry, a stat timeout, or a
66-
length mismatch), `buildTransferPropsFromDroppedPaths` reverts the whole batch to the legacy approximate shape
67-
(`fileCount = count`, `folderCount = 0`), which makes the toast composer fall back to flattened file-count wording.
68-
Honest beats half-right — a partial split would misreport.
61+
Drop preparation runs through the shared transfer entry seam (`pane/transfer-entry.ts`), the SAME path F5/F6 and
62+
clipboard paste use, so all three entry points prepare a transfer identically. On drop,
63+
`pane/drag-drop-controller.svelte.ts::handleFileDrop`:
64+
65+
- **Runs the shared destination guard** (`checkTransferDestinationGuard`) FIRST. Dropping onto a read-only volume shows
66+
the same "Read-only device" alert F5 shows (not a copy dialog the backend would later reject); dropping onto a
67+
search-results pane shows the not-a-folder toast. The guard short-circuits before any stat / volume-resolution work.
68+
- **Resolves the REAL source volume** via `resolveSourceVolumeId` (frontend `findVolumeIdForPath` longest-prefix →
69+
backend `resolve_path_volume` for the common parent → honest-unknown `root` default) and passes it to
70+
`buildTransferPropsFromDroppedPaths`. This is what feeds the dialog's byte scan (`startScanPreview`'s `sourceVolumeId`
71+
arg), so an MTP→local / local→MTP drop stats the right volume and the counters fill. The old hardcoded
72+
`sourceVolumeId = destVolumeId` placeholder stat'd the source paths as the wrong shape and reported 0 bytes / 0 files.
73+
`resolveSourceVolumeId` NEVER ships a knowingly-wrong id — when sources span volumes or resolution fails, it returns
74+
`root` (the honest unknown, today's degraded-but-correct behavior).
75+
- **Fetches each dropped path's top-level kind** (file vs. folder) in one batched `stat_paths_kinds` IPC before opening
76+
the confirmation dialog, so both the dialog and the completion toast report the real "N files and M folders" split.
77+
The stat runs under the backend read timeout (2 s) and falls back to all-unknown on a hung mount, so it never blocks
78+
the drop. The split is all-or-nothing: if ANY path's kind is unknown (a virtual MTP/SMB path that landed on the
79+
pasteboard, a vanished entry, a stat timeout, or a length mismatch), `buildTransferPropsFromDroppedPaths` reverts the
80+
whole batch to the legacy approximate shape (`fileCount = count`, `folderCount = 0`), which makes the toast composer
81+
fall back to flattened file-count wording. Honest beats half-right — a partial split would misreport.
6982

7083
### Drag image detection (macOS-specific hack)
7184

apps/desktop/src/lib/file-explorer/pane/CLAUDE.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ list).
5656
| `type-to-jump-keys.ts` | Pure `isTypeToJumpChar` / `isTypeToJumpResetKey` shared by both jump intercepts |
5757
| `initialization.ts` | Load persisted tabs + status + settings; resolve volumes; apply E2E overrides |
5858
| `tab-operations.ts` | Tab CRUD + context menu + persistence wired to `tabs/tab-state-manager` |
59-
| `transfer-operations.ts` | Build `TransferDialogPropsData` (and snapshot variant) from a focused pane |
59+
| `transfer-operations.ts` | Build `TransferDialogPropsData` (and snapshot/dropped variants) from a focused pane |
60+
| `transfer-entry.ts` | Shared transfer entry seam: `checkTransferDestinationGuard` + `resolveSourceVolumeId` |
6061
| `sorting-handlers.ts` | `getNewSortOrder` (column click cycle), `toFrontendIndices` (`..` offset) |
6162
| `index-events.ts` | Throttled `index-dir-updated` handler with `/private/` symlink resolution |
6263
| `navigate.ts` | `navigate(intent, deps)` transaction: the single coordinator-level pane-nav entry |
@@ -129,11 +130,12 @@ string. The guards:
129130
system clipboard, and an MTP-worded toast on a reachable network paste would be a new, mis-worded toast (PR3). On the
130131
live clipboard-time pane id set this is byte-equivalent to the old `startsWith('mtp-')` gate, pinned by the
131132
equivalence test in `clipboard-operations.test.ts`.
132-
- **Transfer / delete** (`file-operation-commands.ts`): source routing (snapshot builder) off `!hasBackendListing`; the
133-
dest-paste block off `!canPasteInto` SCOPED to the `search-results` kind (so the toast wording stays correct — a
134-
network dest has the same `false` cap but historically fell through silently; the capability decides the block, the
135-
kind decides the toast, same split as dispatch). The `isReadOnly` alerts stay per-`VolumeInfo` (Q4), and the
136-
`search-results://` URL parses stay (namespace mechanics).
133+
- **Transfer / delete** (`file-operation-commands.ts`): source routing (snapshot builder) off `!hasBackendListing`. The
134+
destination guards (search-results dest-paste block off `!canPasteInto` scoped to the `search-results` kind so the
135+
toast wording stays correct; the `isReadOnly` alert per-`VolumeInfo`, Q4) live in `transfer-entry.ts`'s
136+
`checkTransferDestinationGuard` so F5/F6, drag-and-drop, AND paste run the identical chain — see
137+
`file-operations/transfer/CLAUDE.md` § "One transfer entry seam". The `search-results://` URL parses stay (namespace
138+
mechanics).
137139
- **`pane-commands.ts`**: `isSnapshotPane` (the Selection-dialog banner flag) off `!hasBackendListing`.
138140
- **MCP sync** (`pane-mcp-sync.svelte.ts`): the network/search skip off `!syncsToMcp`. The deps interface carries a
139141
single `getSyncsToMcp()` accessor (FilePane supplies it from its derived caps); the two `getIs*View()` deps retired.

apps/desktop/src/lib/file-explorer/pane/clipboard-operations.test.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,17 @@ vi.mock('$lib/ui/toast', () => ({ addToast: addToastSpy }))
4141

4242
vi.mock('$lib/search/snapshot-store.svelte', () => ({ resolveSnapshotPaths: resolveSnapshotPathsSpy }))
4343

44-
vi.mock('./transfer-operations', () => ({ getCommonParentPath: getCommonParentPathSpy }))
44+
// `transfer-entry` (the shared guard chain) imports `getDestinationVolumeInfo`
45+
// from here too, so the mock must export it. Keep it a thin lookup matching the
46+
// real one so the read-only paste guard exercises real behavior off the
47+
// per-test volumes list.
48+
vi.mock('./transfer-operations', () => ({
49+
getCommonParentPath: getCommonParentPathSpy,
50+
getDestinationVolumeInfo: (volumeId: string, volumes: { id: string; name: string; isReadOnly?: boolean }[]) => {
51+
const v = volumes.find((vol) => vol.id === volumeId)
52+
return v ? { name: v.name, isReadOnly: v.isReadOnly ?? false } : undefined
53+
},
54+
}))
4555

4656
// The MTP / snapshot refusals read the capability table via `capabilitiesFor`,
4757
// which resolves fsType/category from the volume store for real ids. The two
@@ -85,6 +95,7 @@ interface AccessConfig {
8595
volumeId?: string
8696
path?: string
8797
showHiddenFiles?: boolean
98+
volumes?: { id: string; name: string; isReadOnly?: boolean }[]
8899
}
89100

90101
function buildAccess(config: AccessConfig = {}): PaneAccess {
@@ -97,12 +108,15 @@ function buildAccess(config: AccessConfig = {}): PaneAccess {
97108
getFocusedPane: () => config.focusedPane ?? 'left',
98109
otherPane: (pane) => (pane === 'left' ? 'right' : 'left'),
99110
getShowHiddenFiles: () => config.showHiddenFiles ?? true,
100-
getVolumes: () => [],
111+
getVolumes: () => (config.volumes ?? []) as unknown as ReturnType<PaneAccess['getVolumes']>,
101112
focusContainer: () => {},
102113
}
103114
}
104115

105-
const dialogsStub = { startTransferProgress: vi.fn<(props: TransferProgressPropsData) => void>() }
116+
const dialogsStub = {
117+
startTransferProgress: vi.fn<(props: TransferProgressPropsData) => void>(),
118+
showAlert: vi.fn<(title: string, message: string) => void>(),
119+
}
106120

107121
function buildDialogs() {
108122
return dialogsStub as unknown as Parameters<typeof createClipboardOperations>[1]
@@ -223,6 +237,23 @@ describe('pasteFromClipboard', () => {
223237
expect(dialogsStub.startTransferProgress).not.toHaveBeenCalled()
224238
})
225239

240+
it('refuses pasting into a read-only destination with the shared "Read-only device" alert', async () => {
241+
const access = buildAccess({
242+
volumeId: 'ext-ro',
243+
volumes: [{ id: 'ext-ro', name: 'Backup', isReadOnly: true }],
244+
})
245+
246+
await createClipboardOperations(access, buildDialogs()).pasteFromClipboard(false)
247+
248+
expect(dialogsStub.showAlert).toHaveBeenCalledWith(
249+
'Read-only device',
250+
'"Backup" is read-only. You can copy files from it, but not to it.',
251+
)
252+
// The shared guard fires before reading the clipboard or queueing anything.
253+
expect(readClipboardFilesSpy).not.toHaveBeenCalled()
254+
expect(dialogsStub.startTransferProgress).not.toHaveBeenCalled()
255+
})
256+
226257
it('warns and bails when the clipboard is empty', async () => {
227258
readClipboardFilesSpy.mockResolvedValue({ paths: [], isCut: false })
228259
const access = buildAccess({ volumeId: 'root' })

apps/desktop/src/lib/file-explorer/pane/clipboard-operations.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { getAppLogger } from '$lib/logging/logger'
1313
import { formatNumber } from '$lib/file-explorer/selection/selection-info-utils'
1414
import type { TransferOperationType } from '../types'
1515
import { getCommonParentPath } from './transfer-operations'
16+
import { checkTransferDestinationGuard } from './transfer-entry'
1617
import { capabilitiesFor } from './volume-capabilities'
1718
import type { createDialogState } from './dialog-state.svelte'
1819
import type { PaneAccess } from './pane-access'
@@ -192,13 +193,26 @@ export function createClipboardOperations(access: PaneAccess, dialogs: DialogSta
192193
try {
193194
// Check MTP before reading clipboard; MTP paste is always rejected,
194195
// no point reading the system clipboard just to reject it. The capability
195-
// decides the refusal, not a `startsWith('mtp-')` string (A6).
196+
// decides the refusal, not a `startsWith('mtp-')` string (A6). The
197+
// MTP-specific copy ("Use F5…") stays separate from the shared guard
198+
// because it points the user at the F5/F6 flow MTP paste lacks.
196199
const volumeId = access.getPaneVolumeId(access.getFocusedPane())
197200
if (isMtpClipboardRefusal(volumeId)) {
198201
addToast('Use F5 to copy files to MTP devices', { level: 'info' })
199202
return
200203
}
201204

205+
// Shared destination guard (read-only alert + search-results refusal) —
206+
// the same chain F5/F6 and drag-and-drop run, so pasting into a read-only
207+
// destination gets the same "Read-only device" alert instead of silently
208+
// queueing a transfer the backend would reject.
209+
const guard = checkTransferDestinationGuard(volumeId, access.getVolumes())
210+
if (!guard.ok) {
211+
if (guard.toast) addToast(guard.toast.message, { level: guard.toast.level })
212+
else dialogs.showAlert(guard.alert.title, guard.alert.message)
213+
return
214+
}
215+
202216
const result = await readClipboardFiles()
203217

204218
if (result.paths.length === 0) {

0 commit comments

Comments
 (0)