Skip to content

Commit ab44a72

Browse files
committed
Navigation: migrate every bare-path caller to { location } (staging step 2)
Each edge that fed a bare path into navigation now resolves it to a `Location` (volume id + path) FIRST, so navigation always lands on the right volume — the fix for the search-result-over-SMB bug. Failures surface a friendly toast (or a typed MCP `ok: false`) instead of a wrong-volume listing. Migrated callers: - Search "Go to file" (`handleSearchNavigate`): now delegates to a shared, testable `revealSearchResultInPane` (resolve the parent dir's volume → navigate → move cursor onto the file). A result can live on any volume, so it can't assume the pane's. - MCP `nav_to_path`: resolves the path first, then routes `{ location }`; unresolvable → typed refusal. The on-network refusal NARROWS — a local target from a network pane now switches and navigates; only an `smb://` target still refuses. - ⌘G "Go to path" and ⌘J "Go to latest download": resolve the per-variant dir on the FE at jump/reveal time (`resolve_go_to_path` stays pure, no per-keystroke statfs). - `mirrorLocalStateToPane`: the cross-volume/in-place split folds into `{ location }`; the same-volume same-path no-op is preserved (routing it through `{ location }` would reload the listing). - Search-results row activation (`FilePane`): resolving a real entry now bubbles a `Location` via a new `onGoToLocation` callback (the go-to-a-location intent, distinct from `onVolumeChange`'s volume-(re)select), gated on the existing `isSearchResultsView` A6 classifier. Unresolvable now shows the shared toast (was a silent `log.warn`). Drops `FilePane.switchVolumeForRealPath` and its `isCrossVolumeNavigation` use. Single-sources the resolve-or-toast as `resolveLocationOrToast` in `navigate-and-select.ts`, reused by the ⌘G, ⌘J, search, and snapshot-row edges. New shared string `fileExplorer.navigation.locationUnreachableToast`. Tests: the search edge (resolve + reveal, failure → toast), MCP `nav_to_path` (resolve → navigate, unresolvable → ok:false, refusal verbatim), and updated ⌘G / ⌘J edge tests. The bare `{ path }` arm still exists; its removal is the final step.
1 parent b029c43 commit ab44a72

13 files changed

Lines changed: 417 additions & 129 deletions

File tree

apps/desktop/src/lib/downloads/go-to-latest.test.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
downloadsWatcherStatusMock,
1010
addToastMock,
1111
openPrivacySettingsMock,
12+
resolveLocationMock,
1213
navigateMock,
1314
moveCursorMock,
1415
getFocusedPaneMock,
@@ -19,6 +20,8 @@ const {
1920
downloadsWatcherStatusMock: vi.fn(),
2021
addToastMock: vi.fn(() => 'toast-id'),
2122
openPrivacySettingsMock: vi.fn(() => Promise.resolve()),
23+
// The download's parent dir resolves to a `Location` on the `root` volume.
24+
resolveLocationMock: vi.fn((path: string) => Promise.resolve({ ok: true as const, location: { volumeId: 'root', path } })),
2225
// `navigate()` returns a `NavigateResult`; default to a started no-op.
2326
navigateMock: vi.fn((): NavigateResult => ({ status: 'started', settled: Promise.resolve() })),
2427
moveCursorMock: vi.fn(() => Promise.resolve()),
@@ -49,6 +52,10 @@ vi.mock('$lib/tauri-commands', () => ({
4952
openPrivacySettings: openPrivacySettingsMock,
5053
}))
5154

55+
vi.mock('$lib/file-explorer/navigation/resolve-location', () => ({
56+
resolveLocation: resolveLocationMock,
57+
}))
58+
5259
// Component imports return opaque module refs; we only assert that the same
5360
// reference is passed to `addToast` (proves the dedup id wires the right toast).
5461
vi.mock('./LatestDownloadEmptyToastContent.svelte', () => ({
@@ -68,6 +75,12 @@ import LatestDownloadEmptyToastContent from './LatestDownloadEmptyToastContent.s
6875
import LatestDownloadFdaToastContent from './LatestDownloadFdaToastContent.svelte'
6976
import type { ExplorerAPI } from '../../routes/(main)/explorer-api'
7077

78+
/** Flush the queued microtasks (the async `onGoToDownloads` resolve-then-navigate). */
79+
async function flushMicrotasks(): Promise<void> {
80+
await Promise.resolve()
81+
await Promise.resolve()
82+
}
83+
7184
function makeExplorerStub(): ExplorerAPI {
7285
// Only the methods these helpers touch need real stubs. Everything else is
7386
// unused; the typed stub avoids a `Partial<ExplorerAPI>` cast leaking into the
@@ -102,6 +115,9 @@ describe('goToLatestDownload', () => {
102115
downloadsWatcherStatusMock.mockReset()
103116
addToastMock.mockReset().mockReturnValue('toast-id')
104117
openPrivacySettingsMock.mockReset().mockResolvedValue(undefined)
118+
resolveLocationMock
119+
.mockReset()
120+
.mockImplementation((path: string) => Promise.resolve({ ok: true as const, location: { volumeId: 'root', path } }))
105121
navigateMock.mockReset().mockReturnValue({ status: 'started', settled: Promise.resolve() })
106122
moveCursorMock.mockReset().mockResolvedValue(undefined)
107123
getFocusedPaneMock.mockReset().mockReturnValue('left')
@@ -122,7 +138,7 @@ describe('goToLatestDownload', () => {
122138

123139
await goToLatestDownload(makeExplorerStub())
124140

125-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'right', to: { path: '/Users/me/Downloads' }, source: 'user' })
141+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'right', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
126142
expect(moveCursorMock).toHaveBeenCalledWith('right', 'report.pdf')
127143
expect(addToastMock).not.toHaveBeenCalled()
128144
})
@@ -167,7 +183,7 @@ describe('goToLatestDownload', () => {
167183

168184
await goToLatestDownload(makeExplorerStub())
169185

170-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
186+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
171187
expect(setFocusedPaneMock).not.toHaveBeenCalled()
172188
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
173189
})
@@ -189,7 +205,7 @@ describe('goToLatestDownload', () => {
189205

190206
await goToLatestDownload(makeExplorerStub())
191207

192-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
208+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
193209
expect(setFocusedPaneMock).not.toHaveBeenCalled()
194210
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
195211
})
@@ -210,7 +226,7 @@ describe('goToLatestDownload', () => {
210226

211227
await goToLatestDownload(makeExplorerStub())
212228

213-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
229+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
214230
expect(setFocusedPaneMock).not.toHaveBeenCalled()
215231
})
216232

@@ -239,9 +255,11 @@ describe('goToLatestDownload', () => {
239255
// The "Go to Downloads" handler arrives as a prop (snapshotted closure
240256
// over the focused pane + Downloads dir), not via a module-state shim.
241257
expect(typeof options.props?.onGoToDownloads).toBe('function')
242-
// Invoking the prop triggers the snapshotted navigation.
258+
// Invoking the prop triggers the snapshotted navigation (which resolves the
259+
// dir's volume first, so flush the microtasks before asserting).
243260
options.props?.onGoToDownloads()
244-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
261+
await flushMicrotasks()
262+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
245263
expect(moveCursorMock).not.toHaveBeenCalled()
246264
})
247265

@@ -260,6 +278,7 @@ describe('goToLatestDownload', () => {
260278
// action must re-evaluate which pane shows the dir at CLICK time.
261279
paneShows('right', '/Users/me/Downloads')
262280
options.props?.onGoToDownloads()
281+
await flushMicrotasks()
263282

264283
// Pane reuse: focus the pane that shows Downloads, no fresh navigation.
265284
expect(navigateMock).not.toHaveBeenCalled()
@@ -335,6 +354,9 @@ describe('goToLatestDownload', () => {
335354
describe('goToDownload', () => {
336355
beforeEach(() => {
337356
addToastMock.mockReset().mockReturnValue('toast-id')
357+
resolveLocationMock
358+
.mockReset()
359+
.mockImplementation((path: string) => Promise.resolve({ ok: true as const, location: { volumeId: 'root', path } }))
338360
navigateMock.mockReset().mockReturnValue({ status: 'started', settled: Promise.resolve() })
339361
moveCursorMock.mockReset().mockResolvedValue(undefined)
340362
getFocusedPaneMock.mockReset().mockReturnValue('left')
@@ -345,7 +367,7 @@ describe('goToDownload', () => {
345367
it('navigates the focused pane to parentDir and selects the file when neither pane shows it', async () => {
346368
await goToDownload(makeExplorerStub(), '/Users/me/Downloads', 'report.pdf')
347369

348-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
370+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
349371
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
350372
})
351373

@@ -375,7 +397,7 @@ describe('goToDownload', () => {
375397

376398
await goToDownload(makeExplorerStub(), '/Users/me/Downloads', 'report.pdf')
377399

378-
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
400+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { location: { volumeId: 'root', path: '/Users/me/Downloads' } }, source: 'user' })
379401
expect(moveCursorMock).not.toHaveBeenCalled()
380402
})
381403
})

apps/desktop/src/lib/downloads/go-to-latest.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
import { commands } from '$lib/ipc/bindings'
1414
import { addToast } from '$lib/ui/toast'
1515
import { getAppLogger } from '$lib/logging/logger'
16-
import { revealFileInBestPane, navigateToDirInBestPane } from '$lib/file-explorer/navigation/navigate-and-select'
16+
import {
17+
revealFileInBestPane,
18+
navigateToDirInBestPane,
19+
resolveLocationOrToast,
20+
} from '$lib/file-explorer/navigation/navigate-and-select'
1721
import type { ExplorerAPI } from '../../routes/(main)/explorer-api'
1822

1923
import LatestDownloadEmptyToastContent from './LatestDownloadEmptyToastContent.svelte'
@@ -47,7 +51,7 @@ export async function goToLatestDownload(explorer: ExplorerAPI | undefined): Pro
4751

4852
const result = await commands.goToLatestDownload()
4953
if (result.status === 'ok') {
50-
await revealFileInBestPane(explorer, result.data.parentDir, result.data.fileName)
54+
await revealDownloadInBestPane(explorer, result.data.parentDir, result.data.fileName)
5155
return
5256
}
5357

@@ -87,7 +91,19 @@ export async function goToDownload(
8791
log.debug('goToDownload: no explorer; skipping (HMR or pre-mount)')
8892
return
8993
}
90-
await revealFileInBestPane(explorer, parentDir, fileName)
94+
await revealDownloadInBestPane(explorer, parentDir, fileName)
95+
}
96+
97+
/**
98+
* Resolve the download's parent dir to a `Location` (volume id + path), then
99+
* reveal the file in the best pane. A download is local, so this resolves to a
100+
* real local volume; if it can't (drive gone), show the shared
101+
* "couldn't reach that location's drive" toast instead of navigating wrong.
102+
*/
103+
async function revealDownloadInBestPane(explorer: ExplorerAPI, parentDir: string, fileName: string): Promise<void> {
104+
const location = await resolveLocationOrToast(parentDir)
105+
if (!location) return
106+
await revealFileInBestPane(explorer, location, fileName)
91107
}
92108

93109
async function showEmptyToast(explorer: ExplorerAPI): Promise<void> {
@@ -104,7 +120,13 @@ async function showEmptyToast(explorer: ExplorerAPI): Promise<void> {
104120
log.warn('Go to Downloads pressed but Downloads dir is unresolved; no action')
105121
return
106122
}
107-
void navigateToDirInBestPane(explorer, downloadsDir)
123+
void (async () => {
124+
// Resolve the Downloads dir's volume at click time (its drive could have
125+
// changed while the toast sat there), then navigate to the best pane.
126+
const location = await resolveLocationOrToast(downloadsDir)
127+
if (!location) return
128+
await navigateToDirInBestPane(explorer, location)
129+
})()
108130
}
109131
addToast(LatestDownloadEmptyToastContent, {
110132
id: LATEST_DOWNLOAD_EMPTY_TOAST_ID,

apps/desktop/src/lib/file-explorer/navigation/navigate-and-select.test.ts

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest'
2-
import { navigateToDirInPane, navigateToFileInPane } from './navigate-and-select'
2+
3+
const { resolveLocationMock, addToastMock } = vi.hoisted(() => ({
4+
resolveLocationMock: vi.fn(),
5+
addToastMock: vi.fn(() => 'toast-id'),
6+
}))
7+
vi.mock('./resolve-location', () => ({ resolveLocation: resolveLocationMock }))
8+
vi.mock('$lib/ui/toast', () => ({ addToast: addToastMock }))
9+
10+
import { navigateToDirInPane, navigateToFileInPane, revealSearchResultInPane } from './navigate-and-select'
311
import type { ExplorerAPI } from '../../../routes/(main)/explorer-api'
412
import type { NavigateResult } from '../pane/navigate'
13+
import type { Location } from '$lib/tauri-commands'
14+
15+
/** A `Location` (volume id + path) — the resolved input these helpers now take. */
16+
function loc(path: string, volumeId = 'root'): Location {
17+
return { volumeId, path }
18+
}
519

620
/** A `started` result whose `settled` is the given promise (defaults to resolved). */
721
function started(settled: Promise<void> = Promise.resolve()): NavigateResult {
@@ -21,16 +35,16 @@ function makeExplorer(navResult: NavigateResult) {
2135
}
2236

2337
describe('navigateToDirInPane', () => {
24-
it('navigates to the dir and never moves the cursor', async () => {
38+
it('navigates to the dir location and never moves the cursor', async () => {
2539
const { explorer, navigate, moveCursor } = makeExplorer(started())
26-
await navigateToDirInPane(explorer, 'left', '/tmp/here')
27-
expect(navigate).toHaveBeenCalledWith({ pane: 'left', to: { path: '/tmp/here' }, source: 'user' })
40+
await navigateToDirInPane(explorer, 'left', loc('/tmp/here'))
41+
expect(navigate).toHaveBeenCalledWith({ pane: 'left', to: { location: loc('/tmp/here') }, source: 'user' })
2842
expect(moveCursor).not.toHaveBeenCalled()
2943
})
3044

3145
it('bails on a refusal without throwing', async () => {
3246
const { explorer, moveCursor } = makeExplorer(refused('snapshot pane on a missing volume'))
33-
await expect(navigateToDirInPane(explorer, 'right', '/tmp')).resolves.toBeUndefined()
47+
await expect(navigateToDirInPane(explorer, 'right', loc('/tmp'))).resolves.toBeUndefined()
3448
expect(moveCursor).not.toHaveBeenCalled()
3549
})
3650
})
@@ -42,10 +56,10 @@ describe('navigateToFileInPane', () => {
4256
moveCursorCalls = []
4357
})
4458

45-
it('navigates to the parent, then moves the cursor onto the file', async () => {
59+
it('navigates to the parent location, then moves the cursor onto the file', async () => {
4660
const { explorer, navigate, moveCursor } = makeExplorer(started())
47-
await navigateToFileInPane(explorer, 'left', '/tmp', 'a.txt')
48-
expect(navigate).toHaveBeenCalledWith({ pane: 'left', to: { path: '/tmp' }, source: 'user' })
61+
await navigateToFileInPane(explorer, 'left', loc('/tmp'), 'a.txt')
62+
expect(navigate).toHaveBeenCalledWith({ pane: 'left', to: { location: loc('/tmp') }, source: 'user' })
4963
expect(moveCursor).toHaveBeenCalledWith('left', 'a.txt')
5064
})
5165

@@ -61,7 +75,7 @@ describe('navigateToFileInPane', () => {
6175
})
6276
const explorer = { navigate, moveCursor } as unknown as ExplorerAPI
6377

64-
const promise = navigateToFileInPane(explorer, 'left', '/tmp', 'a.txt')
78+
const promise = navigateToFileInPane(explorer, 'left', loc('/tmp'), 'a.txt')
6579
// Cursor must NOT move before the navigation settles.
6680
expect(moveCursorCalls).toHaveLength(0)
6781
resolveListing()
@@ -71,7 +85,45 @@ describe('navigateToFileInPane', () => {
7185

7286
it('bails on a refusal and never moves the cursor', async () => {
7387
const { explorer, moveCursor } = makeExplorer(refused('cannot start'))
74-
await navigateToFileInPane(explorer, 'left', '/tmp', 'a.txt')
88+
await navigateToFileInPane(explorer, 'left', loc('/tmp'), 'a.txt')
89+
expect(moveCursor).not.toHaveBeenCalled()
90+
})
91+
})
92+
93+
describe('revealSearchResultInPane (the search "Go to file" edge)', () => {
94+
beforeEach(() => {
95+
resolveLocationMock.mockReset()
96+
addToastMock.mockReset().mockReturnValue('toast-id')
97+
})
98+
99+
it("resolves the result's PARENT dir, navigates with that location, then moves the cursor onto the file", async () => {
100+
resolveLocationMock.mockResolvedValue({ ok: true, location: loc('/Volumes/Nas/docs', 'nas') })
101+
const { explorer, navigate, moveCursor } = makeExplorer(started())
102+
103+
await revealSearchResultInPane(explorer, 'left', '/Volumes/Nas/docs/report.pdf')
104+
105+
// It resolves the parent dir, not the file path.
106+
expect(resolveLocationMock).toHaveBeenCalledWith('/Volumes/Nas/docs')
107+
expect(navigate).toHaveBeenCalledWith({
108+
pane: 'left',
109+
to: { location: loc('/Volumes/Nas/docs', 'nas') },
110+
source: 'user',
111+
})
112+
expect(moveCursor).toHaveBeenCalledWith('left', 'report.pdf')
113+
expect(addToastMock).not.toHaveBeenCalled()
114+
})
115+
116+
it('shows the friendly toast and does NOT navigate when the parent volume is unresolvable', async () => {
117+
resolveLocationMock.mockResolvedValue({ ok: false, reason: 'no-volume' })
118+
const { explorer, navigate, moveCursor } = makeExplorer(started())
119+
120+
await revealSearchResultInPane(explorer, 'left', '/Volumes/Gone/docs/report.pdf')
121+
122+
expect(navigate).not.toHaveBeenCalled()
75123
expect(moveCursor).not.toHaveBeenCalled()
124+
expect(addToastMock).toHaveBeenCalledWith(
125+
"Couldn't reach that location's drive. It might be disconnected.",
126+
{ level: 'info' },
127+
)
76128
})
77129
})

0 commit comments

Comments
 (0)