From da3e9b98651514e0922620eb0cbf18ebf11b1bd1 Mon Sep 17 00:00:00 2001 From: Elias Stiffel Date: Sat, 23 May 2026 18:43:24 +0200 Subject: [PATCH 1/2] fix: switch active vault when unmounting it via the vault switcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of the fix for #735 — top-level folder creation invisible in multi-workspace mode. The primary route, where the create-folder flow ignored the sidebar selection and always wrote to the legacy `resolvedPath`, is fixed in #728 (selection-aware routing in `handleCreateFolder` and `FolderTree`). This commit fixes the complementary state-machine bug: the vault switcher could leave `vaultPath` pointing at an unmounted vault, so any consumer that still reads `vaultPath` would operate on a vault the user no longer sees. The vault switcher tracks two related paths: - `defaultWorkspacePath` — the workspace the user is currently "on" (used as the destination for new notes, the graph default, etc.). - `vaultPath` — the active vault the switcher tracks; read by the file watcher, folder/view reloaders, and the active-vault fallback in the folder loader. Unchecking a vault in the switcher previously re-routed only `defaultWorkspacePath` to another mounted vault, leaving `vaultPath` pointing at the just-unmounted vault. The two paths drifted apart and every downstream consumer that read `vaultPath` operated on stale state — for example the folder loader's active-vault fallback (when enabled) would silently re-introduce the unmounted vault into the sidebar, and the file watcher would keep listening to it. `applyMountedChange` now also calls `onSwitchVault(nextPath)` when the unmounted vault is the active one, mirroring the existing reroute for `defaultWorkspacePath`. After the change `vaultPath` and mounted state stay consistent for every consumer. `applyMountedChange` is extracted from `VaultMenu.tsx` to a sibling helper module (`vaultMenuMountedChange.ts`) so it can be unit-tested directly without tripping `react-refresh/only-export-components`. Tests: - New `VaultMenu.applyMountedChange.test.ts` covers the four reroute permutations (default+active, active only, default only, neither), the bail-out when no alternative mounted vault exists, and the remount no-op. Fixes: https://github.com/refactoringhq/tolaria/issues/735 Related: https://github.com/refactoringhq/tolaria/pull/728 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../VaultMenu.applyMountedChange.test.ts | 109 ++++++++++++++++++ src/components/status-bar/VaultMenu.tsx | 36 ++---- .../status-bar/vaultMenuMountedChange.ts | 35 ++++++ 3 files changed, 152 insertions(+), 28 deletions(-) create mode 100644 src/components/status-bar/VaultMenu.applyMountedChange.test.ts create mode 100644 src/components/status-bar/vaultMenuMountedChange.ts diff --git a/src/components/status-bar/VaultMenu.applyMountedChange.test.ts b/src/components/status-bar/VaultMenu.applyMountedChange.test.ts new file mode 100644 index 00000000..d4d8db77 --- /dev/null +++ b/src/components/status-bar/VaultMenu.applyMountedChange.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it, vi } from 'vitest' +import { applyMountedChange } from './vaultMenuMountedChange' +import type { VaultOption } from './types' + +function vault(path: string, mounted = true): VaultOption { + return { label: path, path, alias: path.replace('/', ''), mounted, available: true } +} + +interface ChangeOverrides { + defaultPath?: string + vaultPath?: string + path?: string + mounted?: boolean + includedVaults?: VaultOption[] +} + +function callApply(overrides: ChangeOverrides) { + const onSetDefaultWorkspace = vi.fn() + const onSwitchVault = vi.fn() + const onUpdateWorkspaceIdentity = vi.fn() + applyMountedChange({ + defaultPath: overrides.defaultPath ?? '/a', + vaultPath: overrides.vaultPath ?? '/a', + includedVaults: overrides.includedVaults ?? [vault('/a'), vault('/b')], + mounted: overrides.mounted ?? false, + onSetDefaultWorkspace, + onSwitchVault, + onUpdateWorkspaceIdentity, + path: overrides.path ?? '/a', + }) + return { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } +} + +describe('applyMountedChange', () => { + it('reroutes both the default workspace and the active vault when the unmounted vault is both', () => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ + defaultPath: '/a', + vaultPath: '/a', + path: '/a', + }) + + expect(onSetDefaultWorkspace).toHaveBeenCalledWith('/b') + expect(onSwitchVault).toHaveBeenCalledWith('/b') + expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false }) + }) + + it('reroutes the active vault when only the active vault is unmounted (default workspace differs)', () => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ + defaultPath: '/b', + vaultPath: '/a', + path: '/a', + }) + + expect(onSetDefaultWorkspace).not.toHaveBeenCalled() + expect(onSwitchVault).toHaveBeenCalledWith('/b') + expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false }) + }) + + it('reroutes the default workspace when only the default is unmounted (active vault differs)', () => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ + defaultPath: '/a', + vaultPath: '/b', + path: '/a', + }) + + expect(onSetDefaultWorkspace).toHaveBeenCalledWith('/b') + expect(onSwitchVault).not.toHaveBeenCalled() + expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false }) + }) + + it('does not reroute when the unmounted vault is neither default nor active', () => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ + defaultPath: '/a', + vaultPath: '/a', + path: '/b', + includedVaults: [vault('/a'), vault('/b'), vault('/c')], + }) + + expect(onSetDefaultWorkspace).not.toHaveBeenCalled() + expect(onSwitchVault).not.toHaveBeenCalled() + expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/b', { mounted: false }) + }) + + it('bails out without changing anything when no alternative mounted vault exists', () => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ + defaultPath: '/a', + vaultPath: '/a', + path: '/a', + includedVaults: [vault('/a')], + }) + + expect(onSetDefaultWorkspace).not.toHaveBeenCalled() + expect(onSwitchVault).not.toHaveBeenCalled() + expect(onUpdateWorkspaceIdentity).not.toHaveBeenCalled() + }) + + it('only marks the vault unmounted (no reroute) when remounting', () => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ + defaultPath: '/a', + vaultPath: '/a', + path: '/b', + mounted: true, + }) + + expect(onSetDefaultWorkspace).not.toHaveBeenCalled() + expect(onSwitchVault).not.toHaveBeenCalled() + expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/b', { mounted: true }) + }) +}) diff --git a/src/components/status-bar/VaultMenu.tsx b/src/components/status-bar/VaultMenu.tsx index 14bfc79a..5d6715e5 100644 --- a/src/components/status-bar/VaultMenu.tsx +++ b/src/components/status-bar/VaultMenu.tsx @@ -18,6 +18,7 @@ import type { VaultOption } from './types' import { useDismissibleLayer } from './useDismissibleLayer' import { workspaceAliasFromOption, workspaceIdentityFromVault } from '../../utils/workspaces' import { reorderVaultPath, vaultPathList } from '../../utils/vaultOrdering' +import { applyMountedChange } from './vaultMenuMountedChange' interface VaultMenuProps { vaults: VaultOption[] @@ -89,6 +90,7 @@ interface VaultMenuInteractionOptions { onSwitchVault: (path: string) => void onUpdateWorkspaceIdentity?: (path: string, patch: Partial) => void setOpen: (open: boolean) => void + vaultPath: string } interface MountToggleRequest { @@ -103,10 +105,6 @@ interface VaultPathSelection extends VaultMenuInteractionOptions { path: string } -interface VaultMountChangeRequest extends VaultMenuInteractionOptions { - mounted: boolean - path: string -} function getVaultTriggerClassName(open: boolean, compact: boolean) { if (compact) { @@ -297,10 +295,6 @@ function useIncludedVaults(vaults: VaultOption[], defaultPath: string): VaultOpt return useMemo(() => vaults.filter((vault) => isIncludedVault(vault, defaultPath)), [defaultPath, vaults]) } -function nextIncludedVaultPath(includedVaults: VaultOption[], currentPath: string): string | null { - return includedVaults.find((vault) => vault.path !== currentPath)?.path ?? null -} - function shouldDisableMountToggle({ canSetDefaultWorkspace, defaultPath, @@ -325,22 +319,6 @@ function selectVaultPath({ setOpen(false) } -function applyMountedChange({ - defaultPath, - includedVaults, - mounted, - onSetDefaultWorkspace, - onUpdateWorkspaceIdentity, - path, -}: VaultMountChangeRequest): void { - if (!mounted && path === defaultPath) { - const nextDefaultPath = nextIncludedVaultPath(includedVaults, path) - if (!nextDefaultPath) return - onSetDefaultWorkspace?.(nextDefaultPath) - } - onUpdateWorkspaceIdentity?.(path, { mounted }) -} - function useVaultMenuInteractions({ defaultPath, includedVaults, @@ -349,6 +327,7 @@ function useVaultMenuInteractions({ onSwitchVault, onUpdateWorkspaceIdentity, setOpen, + vaultPath, }: VaultMenuInteractionOptions) { const disableMountToggleForPath = useCallback((path: string) => ( shouldDisableMountToggle({ @@ -370,22 +349,22 @@ function useVaultMenuInteractions({ onUpdateWorkspaceIdentity, path, setOpen, + vaultPath, }) - }, [defaultPath, includedVaults, multiWorkspaceEnabled, onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity, setOpen]) + }, [defaultPath, includedVaults, multiWorkspaceEnabled, onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity, setOpen, vaultPath]) const handleMountedChange = useCallback((path: string, mounted: boolean) => { applyMountedChange({ defaultPath, includedVaults, mounted, - multiWorkspaceEnabled, onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity, path, - setOpen, + vaultPath, }) - }, [defaultPath, includedVaults, multiWorkspaceEnabled, onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity, setOpen]) + }, [defaultPath, includedVaults, onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity, vaultPath]) return { disableMountToggleForPath, handleMountedChange, handleSelectVault } } @@ -751,6 +730,7 @@ export function VaultMenu(props: VaultMenuProps) { onSwitchVault, onUpdateWorkspaceIdentity, setOpen, + vaultPath, }) useDismissibleLayer(open, menuRef, () => setOpen(false)) diff --git a/src/components/status-bar/vaultMenuMountedChange.ts b/src/components/status-bar/vaultMenuMountedChange.ts new file mode 100644 index 00000000..0bf6b462 --- /dev/null +++ b/src/components/status-bar/vaultMenuMountedChange.ts @@ -0,0 +1,35 @@ +import type { VaultOption } from './types' + +export interface VaultMountChangeRequest { + defaultPath: string + includedVaults: VaultOption[] + mounted: boolean + onSetDefaultWorkspace?: (path: string) => void + onSwitchVault: (path: string) => void + onUpdateWorkspaceIdentity?: (path: string, patch: Partial) => void + path: string + vaultPath: string +} + +function nextIncludedVaultPath(includedVaults: VaultOption[], currentPath: string): string | null { + return includedVaults.find((vault) => vault.path !== currentPath)?.path ?? null +} + +export function applyMountedChange({ + defaultPath, + includedVaults, + mounted, + onSetDefaultWorkspace, + onSwitchVault, + onUpdateWorkspaceIdentity, + path, + vaultPath, +}: VaultMountChangeRequest): void { + if (!mounted && (path === defaultPath || path === vaultPath)) { + const nextPath = nextIncludedVaultPath(includedVaults, path) + if (!nextPath) return + if (path === defaultPath) onSetDefaultWorkspace?.(nextPath) + if (path === vaultPath) onSwitchVault(nextPath) + } + onUpdateWorkspaceIdentity?.(path, { mounted }) +} From 42c3b938862d145ffc8c01b49db934a8c07bf2af Mon Sep 17 00:00:00 2001 From: Elias Stiffel Date: Sun, 24 May 2026 00:45:01 +0200 Subject: [PATCH 2/2] refactor: group applyMountedChange callbacks under a single field Code review on pr-735 flagged `applyMountedChange` for exceeding the 8-parameter limit. The three effect callbacks (`onSetDefaultWorkspace`, `onSwitchVault`, `onUpdateWorkspaceIdentity`) are now grouped under a single `callbacks` field. The function's destructure surface drops from 8 to 6 fields, and the semantic split between "state + change" and "effects" is now explicit in the type. No behaviour change; the six existing tests pass unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../VaultMenu.applyMountedChange.test.ts | 4 +-- src/components/status-bar/VaultMenu.tsx | 10 ++++--- .../status-bar/vaultMenuMountedChange.ts | 26 ++++++++++--------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/components/status-bar/VaultMenu.applyMountedChange.test.ts b/src/components/status-bar/VaultMenu.applyMountedChange.test.ts index d4d8db77..b83aacdb 100644 --- a/src/components/status-bar/VaultMenu.applyMountedChange.test.ts +++ b/src/components/status-bar/VaultMenu.applyMountedChange.test.ts @@ -23,10 +23,8 @@ function callApply(overrides: ChangeOverrides) { vaultPath: overrides.vaultPath ?? '/a', includedVaults: overrides.includedVaults ?? [vault('/a'), vault('/b')], mounted: overrides.mounted ?? false, - onSetDefaultWorkspace, - onSwitchVault, - onUpdateWorkspaceIdentity, path: overrides.path ?? '/a', + callbacks: { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity }, }) return { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } } diff --git a/src/components/status-bar/VaultMenu.tsx b/src/components/status-bar/VaultMenu.tsx index 5d6715e5..f5c266e0 100644 --- a/src/components/status-bar/VaultMenu.tsx +++ b/src/components/status-bar/VaultMenu.tsx @@ -356,13 +356,15 @@ function useVaultMenuInteractions({ const handleMountedChange = useCallback((path: string, mounted: boolean) => { applyMountedChange({ defaultPath, + vaultPath, includedVaults, mounted, - onSetDefaultWorkspace, - onSwitchVault, - onUpdateWorkspaceIdentity, path, - vaultPath, + callbacks: { + onSetDefaultWorkspace, + onSwitchVault, + onUpdateWorkspaceIdentity, + }, }) }, [defaultPath, includedVaults, onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity, vaultPath]) diff --git a/src/components/status-bar/vaultMenuMountedChange.ts b/src/components/status-bar/vaultMenuMountedChange.ts index 0bf6b462..a70fe92f 100644 --- a/src/components/status-bar/vaultMenuMountedChange.ts +++ b/src/components/status-bar/vaultMenuMountedChange.ts @@ -1,14 +1,18 @@ import type { VaultOption } from './types' -export interface VaultMountChangeRequest { - defaultPath: string - includedVaults: VaultOption[] - mounted: boolean +export interface VaultMountChangeCallbacks { onSetDefaultWorkspace?: (path: string) => void onSwitchVault: (path: string) => void onUpdateWorkspaceIdentity?: (path: string, patch: Partial) => void - path: string +} + +export interface VaultMountChangeRequest { + defaultPath: string vaultPath: string + includedVaults: VaultOption[] + mounted: boolean + path: string + callbacks: VaultMountChangeCallbacks } function nextIncludedVaultPath(includedVaults: VaultOption[], currentPath: string): string | null { @@ -17,19 +21,17 @@ function nextIncludedVaultPath(includedVaults: VaultOption[], currentPath: strin export function applyMountedChange({ defaultPath, + vaultPath, includedVaults, mounted, - onSetDefaultWorkspace, - onSwitchVault, - onUpdateWorkspaceIdentity, path, - vaultPath, + callbacks, }: VaultMountChangeRequest): void { if (!mounted && (path === defaultPath || path === vaultPath)) { const nextPath = nextIncludedVaultPath(includedVaults, path) if (!nextPath) return - if (path === defaultPath) onSetDefaultWorkspace?.(nextPath) - if (path === vaultPath) onSwitchVault(nextPath) + if (path === defaultPath) callbacks.onSetDefaultWorkspace?.(nextPath) + if (path === vaultPath) callbacks.onSwitchVault(nextPath) } - onUpdateWorkspaceIdentity?.(path, { mounted }) + callbacks.onUpdateWorkspaceIdentity?.(path, { mounted }) }