From f2ec2ab55c4c943307771b8ecbaf1bafbd836331 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sat, 7 Mar 2026 11:33:18 +0100 Subject: [PATCH] feat: make MCP restore command always available in Cmd+K The "Install MCP Server" command was only enabled when status was "not_installed", preventing users from re-registering when MCP got removed or broken. Now the command is always available: - Shows "Install MCP Server" when not installed - Shows "Restore MCP Server" when already installed - Added restore/fix/repair keywords for discoverability - Context-aware toast: "installed" vs "restored" - Menu bar label updated to "Restore MCP Server" Co-Authored-By: Claude Opus 4.6 --- src-tauri/src/menu.rs | 2 +- src/hooks/useCommandRegistry.test.ts | 58 ++++++++++++++++++++++++++++ src/hooks/useCommandRegistry.ts | 2 +- src/hooks/useMcpStatus.test.ts | 35 +++++++++++++++-- src/hooks/useMcpStatus.ts | 5 ++- 5 files changed, 95 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index 81c778df..be394793 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -331,7 +331,7 @@ fn build_vault_menu(app: &App) -> MenuResult { let view_changes = MenuItemBuilder::new("View Pending Changes") .id(VAULT_VIEW_CHANGES) .build(app)?; - let install_mcp = MenuItemBuilder::new("Install MCP Server") + let install_mcp = MenuItemBuilder::new("Restore MCP Server") .id(VAULT_INSTALL_MCP) .build(app)?; let reindex = MenuItemBuilder::new("Reindex Vault") diff --git a/src/hooks/useCommandRegistry.test.ts b/src/hooks/useCommandRegistry.test.ts index 2481cbcb..a35ebdd1 100644 --- a/src/hooks/useCommandRegistry.test.ts +++ b/src/hooks/useCommandRegistry.test.ts @@ -197,6 +197,64 @@ describe('groupSortKey', () => { }) }) +describe('install-mcp command', () => { + it('is enabled when mcpStatus is not_installed and handler provided', () => { + const config = makeConfig({ mcpStatus: 'not_installed', onInstallMcp: vi.fn() }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + expect(cmd).toBeDefined() + expect(cmd!.enabled).toBe(true) + expect(cmd!.label).toBe('Install MCP Server') + }) + + it('is enabled when mcpStatus is installed and handler provided (restore use case)', () => { + const config = makeConfig({ mcpStatus: 'installed', onInstallMcp: vi.fn() }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + expect(cmd!.enabled).toBe(true) + expect(cmd!.label).toBe('Restore MCP Server') + }) + + it('is disabled when mcpStatus is checking', () => { + const config = makeConfig({ mcpStatus: 'checking', onInstallMcp: vi.fn() }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + expect(cmd!.enabled).toBe(false) + }) + + it('is disabled when no handler provided', () => { + const config = makeConfig({ mcpStatus: 'not_installed' }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + expect(cmd!.enabled).toBe(false) + }) + + it('has restore keyword for discoverability', () => { + const config = makeConfig({ mcpStatus: 'installed', onInstallMcp: vi.fn() }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + expect(cmd!.keywords).toContain('restore') + expect(cmd!.keywords).toContain('mcp') + expect(cmd!.keywords).toContain('claude') + }) + + it('executes onInstallMcp callback', () => { + const onInstallMcp = vi.fn() + const config = makeConfig({ mcpStatus: 'installed', onInstallMcp }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + cmd!.execute() + expect(onInstallMcp).toHaveBeenCalled() + }) + + it('is in Settings group', () => { + const config = makeConfig({ mcpStatus: 'installed', onInstallMcp: vi.fn() }) + const { result } = renderHook(() => useCommandRegistry(config)) + const cmd = findCommand(result.current, 'install-mcp') + expect(cmd!.group).toBe('Settings') + }) +}) + describe('buildTypeCommands', () => { it('creates new and list commands for each type', () => { const onCreateNoteOfType = vi.fn() diff --git a/src/hooks/useCommandRegistry.ts b/src/hooks/useCommandRegistry.ts index 54ae7dc1..31115ba7 100644 --- a/src/hooks/useCommandRegistry.ts +++ b/src/hooks/useCommandRegistry.ts @@ -258,7 +258,7 @@ export function useCommandRegistry(config: CommandRegistryConfig): CommandAction { id: 'remove-vault', label: 'Remove Vault from List', group: 'Settings', keywords: ['vault', 'remove', 'disconnect', 'hide'], enabled: (vaultCount ?? 0) > 1 && !!onRemoveActiveVault, execute: () => onRemoveActiveVault?.() }, { id: 'restore-getting-started', label: 'Restore Getting Started Vault', group: 'Settings', keywords: ['vault', 'restore', 'demo', 'getting started', 'reset'], enabled: !!isGettingStartedHidden && !!onRestoreGettingStarted, execute: () => onRestoreGettingStarted?.() }, { id: 'check-updates', label: 'Check for Updates', group: 'Settings', keywords: ['update', 'version', 'upgrade', 'release'], enabled: true, execute: () => onCheckForUpdates?.() }, - { id: 'install-mcp', label: 'Install MCP Server', group: 'Settings', keywords: ['mcp', 'claude', 'ai', 'tools', 'install'], enabled: mcpStatus === 'not_installed' && !!onInstallMcp, execute: () => onInstallMcp?.() }, + { id: 'install-mcp', label: mcpStatus === 'installed' ? 'Restore MCP Server' : 'Install MCP Server', group: 'Settings', keywords: ['mcp', 'claude', 'ai', 'tools', 'install', 'restore', 'fix', 'repair'], enabled: mcpStatus !== 'checking' && !!onInstallMcp, execute: () => onInstallMcp?.() }, { id: 'reindex-vault', label: 'Reindex Vault', group: 'Settings', keywords: ['reindex', 'index', 'search', 'rebuild', 'refresh'], enabled: !!onReindexVault, execute: () => onReindexVault?.() }, // Type-aware: "New [Type]" and "List [Type]" diff --git a/src/hooks/useMcpStatus.test.ts b/src/hooks/useMcpStatus.test.ts index 6e2ba2c2..38b357c1 100644 --- a/src/hooks/useMcpStatus.test.ts +++ b/src/hooks/useMcpStatus.test.ts @@ -66,9 +66,10 @@ describe('useMcpStatus', () => { }) it('install action calls register_mcp_tools and updates status', async () => { + // Auto-register fails (e.g. node not found), leaving status as not_installed mockInvoke.mockImplementation((cmd: string) => { if (cmd === 'check_mcp_status') return Promise.resolve('not_installed') - if (cmd === 'register_mcp_tools') return Promise.resolve('registered') + if (cmd === 'register_mcp_tools') return Promise.reject(new Error('no node')) return Promise.resolve(null) }) @@ -76,12 +77,11 @@ describe('useMcpStatus', () => { const { result } = renderHook(() => useMcpStatus('/vault', onToast)) await waitFor(() => { - expect(result.current.mcpStatus).toBe('installed') + expect(result.current.mcpStatus).toBe('not_installed') }) - // Reset to test install action directly + // Now manual install succeeds mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('not_installed') if (cmd === 'register_mcp_tools') return Promise.resolve('registered') return Promise.resolve(null) }) @@ -131,6 +131,33 @@ describe('useMcpStatus', () => { }) }) + it('install action shows restored toast when status was installed', async () => { + // First call: status already installed + mockInvoke.mockImplementation((cmd: string) => { + if (cmd === 'check_mcp_status') return Promise.resolve('installed') + if (cmd === 'register_mcp_tools') return Promise.resolve('updated') + return Promise.resolve(null) + }) + + const onToast = vi.fn() + const { result } = renderHook(() => useMcpStatus('/vault', onToast)) + + await waitFor(() => { + expect(result.current.mcpStatus).toBe('installed') + }) + + // Clear toasts from auto-register + onToast.mockClear() + + // Now manually trigger install (restore) + await act(async () => { + await result.current.installMcp() + }) + + expect(result.current.mcpStatus).toBe('installed') + expect(onToast).toHaveBeenCalledWith('MCP server restored successfully') + }) + it('does not show toast when already registered', async () => { mockInvoke.mockImplementation((cmd: string) => { if (cmd === 'check_mcp_status') return Promise.resolve('installed') diff --git a/src/hooks/useMcpStatus.ts b/src/hooks/useMcpStatus.ts index c95dbcb1..5535ca7a 100644 --- a/src/hooks/useMcpStatus.ts +++ b/src/hooks/useMcpStatus.ts @@ -19,9 +19,11 @@ export function useMcpStatus( onToast: (msg: string) => void, ) { const [status, setStatus] = useState('checking') + const statusRef = useRef(status) const registeredRef = useRef(null) const onToastRef = useRef(onToast) useEffect(() => { onToastRef.current = onToast }) + useEffect(() => { statusRef.current = status }, [status]) // Check MCP status on vault open / vault switch useEffect(() => { @@ -57,11 +59,12 @@ export function useMcpStatus( }, [vaultPath]) const install = useCallback(async () => { + const wasInstalled = statusRef.current === 'installed' setStatus('checking') try { await tauriCall('register_mcp_tools', { vaultPath }) setStatus('installed') - onToastRef.current('MCP server installed successfully') + onToastRef.current(wasInstalled ? 'MCP server restored successfully' : 'MCP server installed successfully') } catch (e) { setStatus('not_installed') onToastRef.current(`MCP install failed: ${e}`)