Skip to content

Commit 3e613ca

Browse files
committed
Bugfix: SMB share no longer mis-loads local paths after volume switch
A stale `onPathChange` arriving after a volume switch was poisoning the new volume's state: `applyPathChange` only guarded the virtual `network` volume, so for real SMB/MTP volumes the foreign path landed via `pushPath` and `saveLastUsedPathForVolume`, corrupting tab state and persisted last-used-path. On the next switch to the same SMB share, `determineNavigationPath` read the poisoned value back and `applyVolumePathCorrection` "corrected" the pane to a local path on the SMB volume, surfacing as `STATUS_OBJECT_PATH_NOT_FOUND` on `Create`. The volume breadcrumb then resolved the local path back to "Macintosh HD" via `resolvePathVolume`, making the UI look schizophrenic. - Extend the `applyPathChange` stale-guard from "network only" to "any volume" via a new `isPathOnVolume` helper. - Filter `determineNavigationPath`'s `lastUsedPath` lookup through `isPathOnVolume` so older corruption can't re-trigger the bug. - Update the gotcha note in `file-explorer/CLAUDE.md` to match the universal guard. - Add unit tests for `isPathOnVolume` and the foreign-`lastUsedPath` case.
1 parent 6bd0f15 commit 3e613ca

4 files changed

Lines changed: 92 additions & 17 deletions

File tree

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,12 +362,17 @@ layout component re-evaluation). If sveltejs/kit#15287 gets fixed, the workaroun
362362

363363
**Stale `onPathChange` from a slow listing can poison a pane after a volume switch.** `FilePane.onPathChange` fires on
364364
`listing-complete` for whatever path the pane was loading. If the user (or a command like "Copy path between panes")
365-
flips the pane to a different volume — especially the virtual `network` volume — between `listing-start` and
366-
`listing-complete`, the stale callback lands on a pane whose `volumeId` no longer matches the path it carries.
367-
`applyPathChange` in `DualPaneExplorer` guards against the `network` case (drops paths that don't start with `smb://`)
368-
because `pushPath` inherits the current `volumeId` and would otherwise write a malformed `{network, /Volumes/...}`
369-
history entry plus a corrupted `lastUsedPathForVolume('network')`. If you introduce another virtual-volume namespace
370-
with its own non-filesystem prefix, extend the guard.
365+
flips the pane to a different volume between `listing-start` and `listing-complete`, the stale callback lands on a pane
366+
whose `volumeId` no longer matches the path it carries. `applyPathChange` in `DualPaneExplorer` guards against this by
367+
dropping paths that don't belong on the current volume: `smb://`-prefixed for the virtual `network` volume, and via
368+
`isPathOnVolume(path, volumePath)` for every other (real) volume. Without the guard, `pushPath` and
369+
`saveLastUsedPathForVolume` would write a foreign path under the new `volumeId`, corrupting in-memory tab state, the nav
370+
history, and persisted last-used-path state — for example, persisting `/Users/.../project` as the "last used path" for
371+
an SMB share, which would then cause the next switch to that share to attempt a doomed `Create` against
372+
`Users\...\project` and surface as `STATUS_OBJECT_PATH_NOT_FOUND`. `determineNavigationPath` applies the same
373+
`isPathOnVolume` filter when reading back `lastUsedPath` so older corruption from before this guard existed can't
374+
re-trigger the bug. If you introduce another virtual-volume namespace with its own non-filesystem prefix (something
375+
`isPathOnVolume` can't match against), extend the explicit network-style branch.
371376

372377
## Views (`views/`)
373378

apps/desktop/src/lib/file-explorer/navigation/path-navigation.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ vi.mock('$lib/app-status-store', () => ({
1212
const { pathExists } = await import('$lib/tauri-commands')
1313
const { getLastUsedPathForVolume } = await import('$lib/app-status-store')
1414

15-
import { withTimeout, determineNavigationPath } from './path-navigation'
15+
import { withTimeout, determineNavigationPath, isPathOnVolume } from './path-navigation'
1616
import type { OtherPaneState } from './path-navigation'
1717
import { resolveValidPath } from './path-resolution'
1818

@@ -116,6 +116,23 @@ describe('determineNavigationPath', () => {
116116
expect(result).toBe('/Users/test/last-used')
117117
})
118118

119+
it('ignores last used path that is foreign to the target volume', async () => {
120+
// Reproduces the SMB-share-shows-local-path bug: a previously corrupted
121+
// `lastUsedPathForVolume('smb-…-naspi') = '/Users/…/vdavid'` must not be
122+
// returned just because `pathExists` confirms the local path.
123+
mockGetLastUsedPath.mockResolvedValue('/Users/test/local-project')
124+
mockPathExists.mockResolvedValue(true)
125+
126+
const resultPromise = determineNavigationPath('smb-192-168-1-111-445-naspi', '/Volumes/naspi', '/Volumes/naspi', {
127+
otherPaneVolumeId: 'root',
128+
otherPanePath: '/Users/test/local-project',
129+
})
130+
await vi.advanceTimersByTimeAsync(500)
131+
const result = await resultPromise
132+
133+
expect(result).toBe('/Volumes/naspi')
134+
})
135+
119136
it('returns ~ when volume is DEFAULT_VOLUME_ID and no better option', async () => {
120137
mockPathExists.mockResolvedValue(false)
121138
mockGetLastUsedPath.mockResolvedValue(undefined)
@@ -183,6 +200,34 @@ describe('determineNavigationPath', () => {
183200
})
184201
})
185202

203+
describe('isPathOnVolume', () => {
204+
it('accepts an exact match', () => {
205+
expect(isPathOnVolume('/Volumes/naspi', '/Volumes/naspi')).toBe(true)
206+
})
207+
208+
it('accepts a descendant path', () => {
209+
expect(isPathOnVolume('/Volumes/naspi/photos/2024', '/Volumes/naspi')).toBe(true)
210+
})
211+
212+
it('rejects a sibling whose name shares a prefix', () => {
213+
expect(isPathOnVolume('/Volumes/naspi-backup', '/Volumes/naspi')).toBe(false)
214+
})
215+
216+
it('rejects a foreign path', () => {
217+
expect(isPathOnVolume('/Users/test/project', '/Volumes/naspi')).toBe(false)
218+
})
219+
220+
it('treats root volume `/` as matching any absolute path', () => {
221+
expect(isPathOnVolume('/Users/test', '/')).toBe(true)
222+
expect(isPathOnVolume('/', '/')).toBe(true)
223+
})
224+
225+
it('handles a volumePath that already ends in `/`', () => {
226+
expect(isPathOnVolume('smb://host/share/foo', 'smb://')).toBe(true)
227+
expect(isPathOnVolume('/Users/test', 'smb://')).toBe(false)
228+
})
229+
})
230+
186231
describe('resolveValidPath', () => {
187232
beforeEach(() => {
188233
vi.useFakeTimers()

apps/desktop/src/lib/file-explorer/navigation/path-navigation.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ export interface OtherPaneState {
1818
otherPanePath: string
1919
}
2020

21+
/**
22+
* True when `path` equals `volumePath` or is a descendant of it. Used to drop
23+
* stale or corrupted paths that don't belong on the given volume — for example
24+
* a local `/Users/...` path that ended up persisted under an SMB volumeId from
25+
* a previous bug.
26+
*/
27+
export function isPathOnVolume(path: string, volumePath: string): boolean {
28+
if (path === volumePath) return true
29+
const prefix = volumePath.endsWith('/') ? volumePath : volumePath + '/'
30+
return path.startsWith(prefix)
31+
}
32+
2133
/**
2234
* Determines which path to navigate to when switching volumes.
2335
* Runs checks in parallel with 500ms frontend timeouts per check.
@@ -46,7 +58,9 @@ export async function determineNavigationPath(
4658
? withTimeout(pathExists(otherPane.otherPanePath), pathExistsTimeoutMs, false)
4759
: Promise.resolve(false),
4860
getLastUsedPathForVolume(volumeId).then((p) =>
49-
p ? withTimeout(pathExists(p), pathExistsTimeoutMs, false).then((ok) => (ok ? p : null)) : null,
61+
p && isPathOnVolume(p, volumePath)
62+
? withTimeout(pathExists(p), pathExistsTimeoutMs, false).then((ok) => (ok ? p : null))
63+
: null,
5064
),
5165
])
5266

apps/desktop/src/lib/file-explorer/pane/DualPaneExplorer.svelte

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
} from '../types'
4646
import { defaultSortOrders } from '../types'
4747
import { ensureFontMetricsLoaded } from '$lib/font-metrics'
48-
import { determineNavigationPath } from '../navigation/path-navigation'
48+
import { determineNavigationPath, isPathOnVolume } from '../navigation/path-navigation'
4949
import { resolveValidPath } from '../navigation/path-resolution'
5050
5151
import {
@@ -464,15 +464,26 @@
464464
465465
/** Applies a path change to the active tab in-place (the normal non-pinned flow). */
466466
function applyPathChange(pane: 'left' | 'right', path: string) {
467-
// Drop stale `onPathChange` events that arrive after the pane was
468-
// switched to the virtual `network` volume. Without this gate, a
469-
// listing-complete from the previous SMB folder lands after the user
470-
// (or a command like "Copy path between panes") flips to Network, and
471-
// `pushPath` inherits `volumeId='network'` while writing a real path
472-
// — corrupting both `pane.path` and `lastUsedPathForVolume('network')`.
467+
// Drop stale `onPathChange` events that arrive after the pane was switched
468+
// to a different volume. Without this gate, a `listing-complete` from the
469+
// previous folder lands after the user (or a command like "Copy path between
470+
// panes") flips volumes, and `pushPath` / `saveLastUsedPathForVolume` end up
471+
// writing a foreign path under the new `volumeId` — corrupting `pane.path`,
472+
// navigation history, and persisted last-used-path state. Network is checked
473+
// separately because its mount path is the `smb://` scheme, not a filesystem
474+
// prefix that `isPathOnVolume` can match against.
473475
const currentVolumeId = getPaneVolumeId(pane)
474-
if (currentVolumeId === 'network' && !path.startsWith('smb://')) {
475-
log.debug('Dropping stale onPathChange on network pane: {path}', { path })
476+
const currentVolumePath = getPaneVolumePath(pane)
477+
if (currentVolumeId === 'network') {
478+
if (!path.startsWith('smb://')) {
479+
log.debug('Dropping stale onPathChange on network pane: {path}', { path })
480+
return
481+
}
482+
} else if (!isPathOnVolume(path, currentVolumePath)) {
483+
log.debug(
484+
'Dropping stale onPathChange: {path} is not on volume {volumeId} ({volumePath})',
485+
{ path, volumeId: currentVolumeId, volumePath: currentVolumePath },
486+
)
476487
return
477488
}
478489
setPanePath(pane, path)

0 commit comments

Comments
 (0)