Skip to content

Commit ef52db4

Browse files
committed
Navigation: wire navigate(), delete the old braid
- Migrate every coordinator handler + external write-caller onto `navigate(intent): NavigateResult`, then DELETE the four-function braid: `handlePathChange`, `applyPathChange`, `handleVolumeChange`, `applyVolumePathCorrection`, `handleNavigationAction`, `updatePaneAfterHistoryNavigation`, the coordinator `navigateToPath`, `volumeChangeGeneration`, and the `applyPathChange` volume-prefix forensics. - Build the store-backed `NavigateDeps` + a per-pane `tokens` map + a global `correctionGen` holder in `DualPaneExplorer`, mirroring how `paneAccess` is built. `navigate()`'s `commit` is now the only writer of `setPaneVolumeId`/`setPanePath`/`setPaneHistory` outside the five M5 edge-flow handlers and the two network-host history pushes (`handleNetworkHostChange`, `mirrorNetworkStateToPane`). - DPE render callbacks: `onPathChange` → `commitPathFromListing` (+ cursor-restore shim); `onVolumeChange` → `navigate({ volumeId, path })`. Bus `nav.back/forward/parent` → `navigate({ history })`. `mcp-nav-to-path` adapter calls `navigate()` directly and forwards the refusal `message` verbatim as the `mcp-response` error (L12, byte-identical). `navigate-and-select`, `go-to-latest`, `handleSearchNavigate`, `openSearchSnapshotInPane`, `selectVolumeBy*`, and the `copyPathBetweenPanes` mirrors all retire onto `navigate()`. - `ExplorerAPI`: `navigateToPath` + `navigate(action)` collapse into `navigate(intent) => NavigateResult`. - Move the in-place pinned-tab fork (L7) from `navigate()`'s in-place arm to `commitPathFromListing`, so FilePane-internal navigation (Enter on a folder) forks identically to coordinator-initiated nav. The volume-switch fork stays at the `navigate()` call. - Fix two M2-missed behavior differences found via E2E, both adapting `navigate()` (M1 assertions untouched): the cross-volume snapshot arm now swallows a `resolveVolume` rejection into a no-op (matching the old try/catch) so a fire-and-forget `mcp-nav-to-path` can't raise an unhandled rejection; the background path-correction is gated by a SINGLE GLOBAL `correctionGen` (the old `volumeChangeGeneration` was global, not per-pane) so a simultaneous two-pane volume reset can't run both corrections and freeze the webview. Added regression tests for both. - Re-point the M1 regression suites + `navigate-and-select`/`go-to-latest` tests onto the new wiring (plumbing only; refusal strings pinned byte-for-byte). Doc sweep: `snapshot-pane-navigation.ts`, `pane/CLAUDE.md`, and the `commitPathFromListing` doc line (it consults only the foreign-path policy, not the token).
1 parent 6e2fc32 commit ef52db4

15 files changed

Lines changed: 504 additions & 510 deletions

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest'
2+
import type { NavigateResult } from '$lib/file-explorer/pane/navigate'
23

34
// Hoisted mocks: vi.mock factories cannot reference top-level bindings, so we
45
// declare the spy fns inside vi.hoisted() and reuse them from the mock factories
@@ -8,15 +9,16 @@ const {
89
downloadsWatcherStatusMock,
910
addToastMock,
1011
openPrivacySettingsMock,
11-
navigateToPathMock,
12+
navigateMock,
1213
moveCursorMock,
1314
getFocusedPaneMock,
1415
} = vi.hoisted(() => ({
1516
goToLatestDownloadMock: vi.fn(),
1617
downloadsWatcherStatusMock: vi.fn(),
1718
addToastMock: vi.fn(() => 'toast-id'),
1819
openPrivacySettingsMock: vi.fn(() => Promise.resolve()),
19-
navigateToPathMock: vi.fn(() => Promise.resolve()),
20+
// `navigate()` returns a `NavigateResult`; default to a started no-op.
21+
navigateMock: vi.fn((): NavigateResult => ({ status: 'started', settled: Promise.resolve() })),
2022
moveCursorMock: vi.fn(() => Promise.resolve()),
2123
getFocusedPaneMock: vi.fn(() => 'left'),
2224
}))
@@ -61,7 +63,7 @@ function makeExplorerStub(): ExplorerAPI {
6163
// into the helper's call sites.
6264
return {
6365
getFocusedPane: getFocusedPaneMock,
64-
navigateToPath: navigateToPathMock,
66+
navigate: navigateMock,
6567
moveCursor: moveCursorMock,
6668
} as unknown as ExplorerAPI
6769
}
@@ -72,7 +74,7 @@ describe('goToLatestDownload', () => {
7274
downloadsWatcherStatusMock.mockReset()
7375
addToastMock.mockReset().mockReturnValue('toast-id')
7476
openPrivacySettingsMock.mockReset().mockResolvedValue(undefined)
75-
navigateToPathMock.mockReset().mockResolvedValue(undefined)
77+
navigateMock.mockReset().mockReturnValue({ status: 'started', settled: Promise.resolve() })
7678
moveCursorMock.mockReset().mockResolvedValue(undefined)
7779
getFocusedPaneMock.mockReset().mockReturnValue('left')
7880
})
@@ -90,7 +92,7 @@ describe('goToLatestDownload', () => {
9092

9193
await goToLatestDownload(makeExplorerStub())
9294

93-
expect(navigateToPathMock).toHaveBeenCalledWith('right', '/Users/me/Downloads')
95+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'right', to: { path: '/Users/me/Downloads' }, source: 'user' })
9496
expect(moveCursorMock).toHaveBeenCalledWith('right', 'report.pdf')
9597
expect(addToastMock).not.toHaveBeenCalled()
9698
})
@@ -122,7 +124,7 @@ describe('goToLatestDownload', () => {
122124
expect(typeof options.props?.onGoToDownloads).toBe('function')
123125
// Invoking the prop triggers the snapshotted navigation.
124126
options.props?.onGoToDownloads()
125-
expect(navigateToPathMock).toHaveBeenCalledWith('left', '/Users/me/Downloads')
127+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
126128
expect(moveCursorMock).not.toHaveBeenCalled()
127129
})
128130

@@ -188,42 +190,41 @@ describe('goToLatestDownload', () => {
188190

189191
expect(goToLatestDownloadMock).not.toHaveBeenCalled()
190192
expect(addToastMock).not.toHaveBeenCalled()
191-
expect(navigateToPathMock).not.toHaveBeenCalled()
193+
expect(navigateMock).not.toHaveBeenCalled()
192194
})
193195
})
194196

195197
describe('goToDownload', () => {
196198
beforeEach(() => {
197199
addToastMock.mockReset().mockReturnValue('toast-id')
198-
navigateToPathMock.mockReset().mockResolvedValue(undefined)
200+
navigateMock.mockReset().mockReturnValue({ status: 'started', settled: Promise.resolve() })
199201
moveCursorMock.mockReset().mockResolvedValue(undefined)
200202
getFocusedPaneMock.mockReset().mockReturnValue('left')
201203
})
202204

203205
it('navigates the focused pane to parentDir and selects the file', async () => {
204206
await goToDownload(makeExplorerStub(), '/Users/me/Downloads', 'report.pdf')
205207

206-
expect(navigateToPathMock).toHaveBeenCalledWith('left', '/Users/me/Downloads')
208+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
207209
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
208210
})
209211

210212
it('does nothing when the explorer handle is missing (HMR / pre-mount)', async () => {
211213
await goToDownload(undefined, '/Users/me/Downloads', 'report.pdf')
212214

213-
expect(navigateToPathMock).not.toHaveBeenCalled()
215+
expect(navigateMock).not.toHaveBeenCalled()
214216
expect(moveCursorMock).not.toHaveBeenCalled()
215217
})
216218

217-
it('bails out without moving the cursor when navigateToPath refuses synchronously', async () => {
218-
// `navigateToPath` returns `string | Promise<void>`; the mock factory's
219-
// inferred type pins it to `Promise<void>`, so cast for this one return path.
220-
;(navigateToPathMock as unknown as { mockReturnValueOnce: (v: unknown) => void }).mockReturnValueOnce(
221-
'snapshot pane on a missing volume',
222-
)
219+
it('bails out without moving the cursor when navigate refuses synchronously', async () => {
220+
navigateMock.mockReturnValueOnce({
221+
status: 'refused',
222+
reason: { kind: 'no-volume-resolved', message: 'snapshot pane on a missing volume' },
223+
})
223224

224225
await goToDownload(makeExplorerStub(), '/Users/me/Downloads', 'report.pdf')
225226

226-
expect(navigateToPathMock).toHaveBeenCalledWith('left', '/Users/me/Downloads')
227+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
227228
expect(moveCursorMock).not.toHaveBeenCalled()
228229
})
229230
})

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,9 @@ async function showEmptyToast(explorer: ExplorerAPI): Promise<void> {
100100
log.warn('Go to Downloads pressed but Downloads dir is unresolved; no action')
101101
return
102102
}
103-
const result = explorer.navigateToPath(focusedPane, downloadsDir)
104-
if (typeof result === 'string') {
105-
log.warn('Go to Downloads: navigateToPath refused: {reason}', { reason: result })
103+
const result = explorer.navigate({ pane: focusedPane, to: { path: downloadsDir }, source: 'user' })
104+
if (result.status === 'refused') {
105+
log.warn('Go to Downloads: navigate refused: {reason}', { reason: result.reason.message })
106106
}
107107
}
108108
addToast(LatestDownloadEmptyToastContent, {

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

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,35 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest'
22
import { navigateToDirInPane, navigateToFileInPane } from './navigate-and-select'
33
import type { ExplorerAPI } from '../../../routes/(main)/explorer-api'
4+
import type { NavigateResult } from '../pane/navigate'
45

5-
function makeExplorer(navResult: string | Promise<void>) {
6-
const navigateToPath = vi.fn(() => navResult)
6+
/** A `started` result whose `settled` is the given promise (defaults to resolved). */
7+
function started(settled: Promise<void> = Promise.resolve()): NavigateResult {
8+
return { status: 'started', settled }
9+
}
10+
11+
/** A `refused` result carrying the given exact message. */
12+
function refused(message: string): NavigateResult {
13+
return { status: 'refused', reason: { kind: 'no-volume-resolved', message } }
14+
}
15+
16+
function makeExplorer(navResult: NavigateResult) {
17+
const navigate = vi.fn(() => navResult)
718
const moveCursor = vi.fn(() => Promise.resolve())
8-
const explorer = { navigateToPath, moveCursor } as unknown as ExplorerAPI
9-
return { explorer, navigateToPath, moveCursor }
19+
const explorer = { navigate, moveCursor } as unknown as ExplorerAPI
20+
return { explorer, navigate, moveCursor }
1021
}
1122

1223
describe('navigateToDirInPane', () => {
1324
it('navigates to the dir and never moves the cursor', async () => {
14-
const { explorer, navigateToPath, moveCursor } = makeExplorer(Promise.resolve())
25+
const { explorer, navigate, moveCursor } = makeExplorer(started())
1526
await navigateToDirInPane(explorer, 'left', '/tmp/here')
16-
expect(navigateToPath).toHaveBeenCalledWith('left', '/tmp/here')
27+
expect(navigate).toHaveBeenCalledWith({ pane: 'left', to: { path: '/tmp/here' }, source: 'user' })
1728
expect(moveCursor).not.toHaveBeenCalled()
1829
})
1930

20-
it('bails on the sync-error string without throwing', async () => {
21-
const { explorer, moveCursor } = makeExplorer('snapshot pane on a missing volume')
31+
it('bails on a refusal without throwing', async () => {
32+
const { explorer, moveCursor } = makeExplorer(refused('snapshot pane on a missing volume'))
2233
await expect(navigateToDirInPane(explorer, 'right', '/tmp')).resolves.toBeUndefined()
2334
expect(moveCursor).not.toHaveBeenCalled()
2435
})
@@ -32,34 +43,34 @@ describe('navigateToFileInPane', () => {
3243
})
3344

3445
it('navigates to the parent, then moves the cursor onto the file', async () => {
35-
const { explorer, navigateToPath, moveCursor } = makeExplorer(Promise.resolve())
46+
const { explorer, navigate, moveCursor } = makeExplorer(started())
3647
await navigateToFileInPane(explorer, 'left', '/tmp', 'a.txt')
37-
expect(navigateToPath).toHaveBeenCalledWith('left', '/tmp')
48+
expect(navigate).toHaveBeenCalledWith({ pane: 'left', to: { path: '/tmp' }, source: 'user' })
3849
expect(moveCursor).toHaveBeenCalledWith('left', 'a.txt')
3950
})
4051

41-
it('awaits the listing before moving the cursor', async () => {
52+
it('awaits the navigation settle before moving the cursor', async () => {
4253
let resolveListing!: () => void
4354
const listing = new Promise<void>((resolve) => {
4455
resolveListing = resolve
4556
})
46-
const navigateToPath = vi.fn(() => listing)
57+
const navigate = vi.fn(() => started(listing))
4758
const moveCursor = vi.fn(() => {
4859
moveCursorCalls.push(['called'])
4960
return Promise.resolve()
5061
})
51-
const explorer = { navigateToPath, moveCursor } as unknown as ExplorerAPI
62+
const explorer = { navigate, moveCursor } as unknown as ExplorerAPI
5263

5364
const promise = navigateToFileInPane(explorer, 'left', '/tmp', 'a.txt')
54-
// Cursor must NOT move before the listing settles.
65+
// Cursor must NOT move before the navigation settles.
5566
expect(moveCursorCalls).toHaveLength(0)
5667
resolveListing()
5768
await promise
5869
expect(moveCursorCalls).toHaveLength(1)
5970
})
6071

61-
it('bails on the sync-error string and never moves the cursor', async () => {
62-
const { explorer, moveCursor } = makeExplorer('cannot start')
72+
it('bails on a refusal and never moves the cursor', async () => {
73+
const { explorer, moveCursor } = makeExplorer(refused('cannot start'))
6374
await navigateToFileInPane(explorer, 'left', '/tmp', 'a.txt')
6475
expect(moveCursor).not.toHaveBeenCalled()
6576
})

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@
22
* Shared navigation primitives for the "jump somewhere in the focused pane"
33
* features: "Go to latest download" (⌘J) and "Go to path" (⌘G). Both want to
44
* point a pane at a directory and (for files) land the cursor on a specific
5-
* entry, with the same careful handling of `navigateToPath`'s
6-
* `string | Promise<void>` return.
5+
* entry, with the same careful handling of `navigate()`'s `NavigateResult`.
76
*
8-
* `navigateToPath` returns a sync error string when navigation can't even
9-
* start (a snapshot pane on a missing volume, etc.); otherwise it returns a
10-
* Promise that settles when the listing completes. We await the Promise but
11-
* report-and-bail on the sync-error string — without the listing settled,
12-
* `moveCursor` would race against an empty cache.
7+
* `navigate()` returns `{ status: 'refused', reason }` when navigation can't
8+
* even start (a snapshot pane on a missing volume, the network/MTP refusals);
9+
* otherwise `{ status: 'started', settled }` where `settled` resolves when the
10+
* navigation settles. We await `settled` but report-and-bail on a refusal —
11+
* without the listing settled, `moveCursor` would race against an empty cache.
12+
*
13+
* NOTE (L2-adjacent): for the cross-volume snapshot arm, `settled` resolves
14+
* BEFORE the new listing loads (it resolves when the volume-switch commit is
15+
* done). `navigateToFileInPane` relies on `moveCursor`'s own internal
16+
* `whenLoadSettles` to bridge that gap — `settled` is the navigation-started
17+
* gate, `whenLoadSettles` is the real listing gate. Don't try to collapse them.
1318
*/
1419

1520
import { getAppLogger } from '$lib/logging/logger'
@@ -24,16 +29,16 @@ type Pane = 'left' | 'right'
2429
* navigation lands the cursor on the 0th row (`..`).
2530
*/
2631
export async function navigateToDirInPane(explorer: ExplorerAPI, pane: Pane, dir: string): Promise<void> {
27-
const navResult = explorer.navigateToPath(pane, dir)
28-
if (typeof navResult === 'string') {
29-
log.warn('navigateToDirInPane: navigateToPath refused {pane} {dir}: {reason}', {
32+
const result = explorer.navigate({ pane, to: { path: dir }, source: 'user' })
33+
if (result.status === 'refused') {
34+
log.warn('navigateToDirInPane: navigate refused {pane} {dir}: {reason}', {
3035
pane,
3136
dir,
32-
reason: navResult,
37+
reason: result.reason.message,
3338
})
3439
return
3540
}
36-
await navResult
41+
await result.settled
3742
}
3843

3944
/**
@@ -46,15 +51,15 @@ export async function navigateToFileInPane(
4651
parentDir: string,
4752
fileName: string,
4853
): Promise<void> {
49-
const navResult = explorer.navigateToPath(pane, parentDir)
50-
if (typeof navResult === 'string') {
51-
log.warn('navigateToFileInPane: navigateToPath refused {pane} {parentDir}: {reason}', {
54+
const result = explorer.navigate({ pane, to: { path: parentDir }, source: 'user' })
55+
if (result.status === 'refused') {
56+
log.warn('navigateToFileInPane: navigate refused {pane} {parentDir}: {reason}', {
5257
pane,
5358
parentDir,
54-
reason: navResult,
59+
reason: result.reason.message,
5560
})
5661
return
5762
}
58-
await navResult
63+
await result.settled
5964
await explorer.moveCursor(pane, fileName)
6065
}

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ list).
5858
| `transfer-operations.ts` | Build `TransferDialogPropsData` (and snapshot variant) from a focused pane |
5959
| `sorting-handlers.ts` | `getNewSortOrder` (column click cycle), `toFrontendIndices` (`..` offset) |
6060
| `index-events.ts` | Throttled `index-dir-updated` handler with `/private/` symlink resolution |
61+
| `navigate.ts` | `navigate(intent, deps)` transaction: the single coordinator-level pane-nav entry |
6162
| `snapshot-pane-navigation.ts` | `isCrossVolumeNavigation` — snapshot-volume → real-path triggers volume switch |
6263
| `has-parent.ts` | `computeHasParent({ isSearchResultsView, currentPath, effectiveVolumeRoot })` |
6364
| `search-results-keys.ts` | Pure key→action dispatch for the flat snapshot pane |
@@ -95,11 +96,12 @@ poisons the pane with `volumeId === 'search-results'` + real path.
9596
(`clipboard-operations`, `file-operation-commands`, `pane-commands`) that take a `PaneAccess` (live-reference read API)
9697
plus the dialog state. The component keeps one-line `export function` delegates so the `ExplorerAPI` surface is
9798
unchanged. Read-only / delegating bodies move; functions that WRITE component navigation state (`switchPane`,
98-
`swapPanes`, `toggleHiddenFiles`, `setViewMode`, `navigate`, `setSort*`, `navigateToPath`, `moveCursor`,
99-
`selectVolumeBy*`, `copyPathBetweenPanes`, the `mirror*`/`restoreFocus` helpers) stay in the component — un-trapping
100-
that state is the explorer-store phase, not this factoring. `moveCursorByName*` and `validateMtpNavigation` moved even
101-
though they're called from component-resident writers (`moveCursor`, `restoreCursorByFilename`, `navigateToPath`); those
102-
callers reach back via `paneCommands.*`.
99+
`swapPanes`, `toggleHiddenFiles`, `setViewMode`, `navigate`, `setSort*`, `moveCursor`, `selectVolumeBy*`,
100+
`copyPathBetweenPanes`, the `mirror*`/`restoreFocus` helpers) stay in the component — un-trapping that state is the
101+
explorer-store phase, not this factoring. The `navigate(intent)` transaction itself lives in `navigate.ts` (the
102+
component builds its `NavigateDeps` and wraps it as the `navigate` export). `moveCursorByName*` and
103+
`validateMtpNavigation` moved even though they're called from component-resident writers (`moveCursor`,
104+
`restoreCursorByFilename`); those callers reach back via `paneCommands.*`.
103105

104106
**Explorer store (`explorer-state.svelte.ts`).** Module store owning the dual-pane navigation + UI-chrome state that
105107
`DualPaneExplorer` used to trap in component closures: `focusedPane`, `showHiddenFiles`, `leftPaneWidthPercent`, and the
@@ -196,10 +198,10 @@ other. See parent § "Live disk space".
196198
`PaneState.typeToJump` whenever the buffer or indicator is live, so MCP-driven E2E can assert without DOM poking. See
197199
`src-tauri/src/mcp/CLAUDE.md` § State stores.
198200

199-
**Don't add `cd`-style heuristics in `applyPathChange`.** Stale `onPathChange` from a slow listing is dropped by the
200-
volume guard in `DualPaneExplorer.applyPathChange` (`smb://` prefix for `network`, `search-results://` prefix for
201-
snapshots, `isPathOnVolume` for everything else). Adding a new virtual-volume namespace? Extend the explicit prefix
202-
branch. See parent § "Gotchas".
201+
**Don't add `cd`-style heuristics in `commitPathFromListing`.** Stale `onPathChange` from a slow listing is dropped by
202+
the drop-foreign-listings policy in `navigate.ts::commitPathFromListing` (`smb://` prefix for `network`,
203+
`search-results://` prefix for snapshots, `isPathOnVolume` for everything else). Adding a new virtual-volume namespace?
204+
Extend the explicit prefix branch. See parent § "Gotchas".
203205

204206
## Gotchas
205207

0 commit comments

Comments
 (0)