diff --git a/apps/desktop/src-tauri/src/commands/server_manager.rs b/apps/desktop/src-tauri/src/commands/server_manager.rs index 58801b5d..91715514 100644 --- a/apps/desktop/src-tauri/src/commands/server_manager.rs +++ b/apps/desktop/src-tauri/src/commands/server_manager.rs @@ -137,8 +137,9 @@ pub async fn enable_server_v2( Some(app_state.data_dir()), ); - // Attempt connection (manual connect from user clicking Connect button) - let ctx = ConnectionContext::new(space_uuid, server_id.clone(), transport); + // Attempt connection with auto_reconnect=true to avoid starting OAuth flow + // If OAuth is needed, we just set AuthRequired and let user click Connect + let ctx = ConnectionContext::auto(space_uuid, server_id.clone(), transport); let result = pool_service.connect_server(&ctx).await; match result { @@ -148,13 +149,9 @@ pub async fn enable_server_v2( Ok(()) } ConnectionResult::OAuthRequired { .. } => { - // OAuth is needed - set state to AuthRequired (NOT Authenticating) - // Don't open browser yet - wait for user to click Connect - // Cancel the OAuth flow that was started during connection probe - pool_service - .oauth_manager() - .cancel_flow_for_space(space_uuid, &server_id); - + // OAuth is needed - set state to AuthRequired + // auto_reconnect=true prevented OAuth flow from starting, so no cancel needed + // User will click "Connect" to start the actual OAuth flow manager.set_auth_required(&key, None).await; // Mark features unavailable - not connected diff --git a/apps/desktop/src/components/ServerDefinitionModal.tsx b/apps/desktop/src/components/ServerDefinitionModal.tsx new file mode 100644 index 00000000..8d24158e --- /dev/null +++ b/apps/desktop/src/components/ServerDefinitionModal.tsx @@ -0,0 +1,145 @@ +import { useState, useEffect, useCallback } from 'react'; +import { X, Copy, Check, Loader2 } from 'lucide-react'; +import Editor from '@monaco-editor/react'; +import type { ServerViewModel, ServerDefinition } from '../types/registry'; + +interface ServerDefinitionModalProps { + server: ServerViewModel; + onClose: () => void; +} + +/** Extract only ServerDefinition fields, stripping runtime state */ +function extractDefinition(server: ServerViewModel): ServerDefinition { + const { + is_installed: _a, + enabled: _b, + oauth_connected: _c, + input_values: _d, + connection_status: _e, + missing_required_inputs: _f, + last_error: _g, + created_at: _h, + installation_source: _i, + env_overrides: _j, + args_append: _k, + extra_headers: _l, + ...definition + } = server; + return definition; +} + +export function ServerDefinitionModal({ server, onClose }: ServerDefinitionModalProps) { + const [copied, setCopied] = useState(false); + const [editorReady, setEditorReady] = useState(false); + + const definition = extractDefinition(server); + const json = JSON.stringify(definition, null, 2); + + useEffect(() => { + const timer = setTimeout(() => setEditorReady(true), 100); + return () => clearTimeout(timer); + }, []); + + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + onClose(); + } + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, [onClose]); + + const handleCopy = useCallback(async () => { + try { + await navigator.clipboard.writeText(json); + setCopied(true); + setTimeout(() => setCopied(false), 2000); + } catch { + // Fallback for environments where clipboard API is unavailable + } + }, [json]); + + return ( +
+
+ {/* Header */} +
+
+

+ {server.name} +

+

+ Server Definition +

+
+
+ + +
+
+ + {/* Editor Area */} +
+ {!editorReady ? ( +
+ +
+ ) : ( + + +
+ } + /> + )} +
+
+ + ); +} diff --git a/apps/desktop/src/features/registry/ServerDetailModal.tsx b/apps/desktop/src/features/registry/ServerDetailModal.tsx index 95339af8..860fb7af 100644 --- a/apps/desktop/src/features/registry/ServerDetailModal.tsx +++ b/apps/desktop/src/features/registry/ServerDetailModal.tsx @@ -2,8 +2,11 @@ * Server detail modal for viewing full server information. */ +import { useState } from 'react'; +import { Code } from 'lucide-react'; import type { ServerViewModel } from '../../types/registry'; import { ServerIcon } from '../../components/ServerIcon'; +import { ServerDefinitionModal } from '../../components/ServerDefinitionModal'; interface ServerDetailModalProps { server: ServerViewModel; @@ -20,6 +23,8 @@ export function ServerDetailModal({ onUninstall, isLoading, }: ServerDetailModalProps) { + const [showDefinition, setShowDefinition] = useState(false); + return (
{/* Backdrop */} @@ -360,6 +365,13 @@ export function ServerDetailModal({ {/* Footer */}
+
+ + {showDefinition && ( + setShowDefinition(false)} + /> + )}
); } diff --git a/apps/desktop/src/features/servers/ServerActionMenu.tsx b/apps/desktop/src/features/servers/ServerActionMenu.tsx index 0f1ae61f..bbec31e9 100644 --- a/apps/desktop/src/features/servers/ServerActionMenu.tsx +++ b/apps/desktop/src/features/servers/ServerActionMenu.tsx @@ -6,11 +6,12 @@ * - Refresh: Quick reconnect with existing credentials * - Reconnect: Logout + re-authenticate (OAuth only) * - View Logs: Open log viewer + * - View Definition: View server definition JSON * - Uninstall: Remove server */ import { useState, useRef, useEffect } from 'react'; -import { MoreVertical, Settings, RefreshCw, RotateCcw, FileText, Trash2 } from 'lucide-react'; +import { MoreVertical, Settings, RefreshCw, RotateCcw, FileText, Code, Trash2 } from 'lucide-react'; export interface ServerActionMenuProps { serverId: string; @@ -23,12 +24,12 @@ export interface ServerActionMenuProps { onRefresh: () => void; onReconnect: () => void; onViewLogs: () => void; + onViewDefinition: () => void; onUninstall: () => void; - disabled?: boolean; } export function ServerActionMenu({ - serverId: _serverId, + serverId, serverName: _serverName, hasInputs, isOAuth, @@ -38,8 +39,8 @@ export function ServerActionMenu({ onRefresh, onReconnect, onViewLogs, + onViewDefinition, onUninstall, - disabled = false, }: ServerActionMenuProps) { const [isOpen, setIsOpen] = useState(false); const menuRef = useRef(null); @@ -88,12 +89,12 @@ export function ServerActionMenu({ @@ -145,11 +146,23 @@ export function ServerActionMenu({ onClick={() => handleAction(onViewLogs)} className="w-full flex items-center gap-2 px-3 py-2 text-sm text-[rgb(var(--foreground))] hover:bg-[rgb(var(--surface-hover))] transition-colors" role="menuitem" + data-testid={`view-logs-${serverId}`} > View Logs + {/* View Definition - always visible */} + + {/* Separator */}
@@ -158,6 +171,7 @@ export function ServerActionMenu({ onClick={() => handleAction(onUninstall)} className="w-full flex items-center gap-2 px-3 py-2 text-sm text-[rgb(var(--error))] hover:bg-[rgb(var(--error))]/10 transition-colors" role="menuitem" + data-testid={`uninstall-menu-${serverId}`} > Uninstall diff --git a/apps/desktop/src/features/servers/ServersPage.tsx b/apps/desktop/src/features/servers/ServersPage.tsx index 52daf44c..5ce47825 100644 --- a/apps/desktop/src/features/servers/ServersPage.tsx +++ b/apps/desktop/src/features/servers/ServersPage.tsx @@ -22,7 +22,8 @@ import { ServerActionMenu } from './ServerActionMenu'; import type { ServerViewModel, ServerDefinition, InstalledServerState, InputDefinition } from '../../types/registry'; import type { ServerFeature } from '@/lib/api/serverFeatures'; import { listServerFeaturesByServer } from '@/lib/api/serverFeatures'; -import type { ConnectionStatus } from '@/lib/api/serverManager'; +import type { ConnectionStatus, ServerStatusResponse } from '@/lib/api/serverManager'; +import { getServerStatuses as fetchServerStatuses } from '@/lib/api/serverManager'; import { useViewSpace } from '@/stores'; import { useServerManager } from '@/hooks/useServerManager'; import { useGatewayEvents, useDomainEvents } from '@/hooks/useDomainEvents'; @@ -30,6 +31,7 @@ import type { GatewayChangedPayload, ServerChangedPayload } from '@/hooks/useDom import type { FeaturesUpdatedEvent } from '@/lib/api/serverManager'; import { ServerLogViewer } from '@/components/ServerLogViewer'; import { ConfigEditorModal } from '@/components/ConfigEditorModal'; +import { ServerDefinitionModal } from '@/components/ServerDefinitionModal'; import { SourceBadge } from '@/components/SourceBadge'; // Helper to merge definitions with states (same as registryStore) @@ -180,6 +182,9 @@ export function ServersPage() { // Log viewer state const [logViewerServer, setLogViewerServer] = useState<{ id: string; name: string } | null>(null); + + // Definition viewer state + const [definitionServer, setDefinitionServer] = useState<{ id: string; name: string } | null>(null); // Config editor state const [editConfigSpace, setEditConfigSpace] = useState<{ id: string; name: string } | null>(null); @@ -289,20 +294,24 @@ export function ServersPage() { setIsLoading(true); // Use allSettled so we can show installed servers even if registry is offline - const [installedResult, gatewayResult, definitionsResult] = await Promise.allSettled([ + const [installedResult, gatewayResult, definitionsResult, statusesResult] = await Promise.allSettled([ import('@/lib/api/registry').then((m) => m.listInstalledServers(viewSpace?.id)), import('@/lib/api/gateway').then((m) => m.getGatewayStatus(viewSpace?.id)), import('@/lib/api/registry').then((m) => m.discoverServers()), + viewSpace?.id ? fetchServerStatuses(viewSpace.id) : Promise.resolve({} as Record), ]); - + // Extract values, using fallbacks for failures const installed = installedResult.status === 'fulfilled' ? installedResult.value : []; - const gateway = gatewayResult.status === 'fulfilled' - ? gatewayResult.value + const gateway = gatewayResult.status === 'fulfilled' + ? gatewayResult.value : { running: false, url: null }; - const definitions = definitionsResult.status === 'fulfilled' - ? definitionsResult.value + const definitions = definitionsResult.status === 'fulfilled' + ? definitionsResult.value : []; + const runtimeStatuses: Record = statusesResult.status === 'fulfilled' + ? statusesResult.value + : {}; // Log if registry is offline but we have installed servers @@ -333,13 +342,27 @@ export function ServersPage() { mergedServers = installed.map(state => createOfflineServerViewModel(state)); } + // Apply runtime statuses from ServerManager to fix initial connection_status + // (mergeDefinitionsWithStates hardcodes 'connecting' for enabled servers) + const mapStatus = (s: ConnectionStatus): ServerViewModel['connection_status'] => { + if (s === 'refreshing' || s === 'authenticating') return 'connecting'; + return s; + }; + for (const server of mergedServers) { + const runtime = runtimeStatuses[server.id]; + if (runtime) { + server.connection_status = mapStatus(runtime.status); + server.last_error = runtime.message || null; + } + } + // Sort by installation time (newest first) mergedServers.sort((a, b) => { const dateA = new Date(a.created_at || 0).getTime(); const dateB = new Date(b.created_at || 0).getTime(); return dateB - dateA; }); - + setInstalledServers(mergedServers); setGatewayRunning(gateway.running); setGatewayUrl(gateway.url); @@ -448,10 +471,12 @@ export function ServersPage() { } // For OAuth servers: show Connect button - if (server.auth?.type === 'oauth') { + // Check both static definition and runtime oauth_connected flag + // (some servers like Sentry declare api_key but actually use OAuth at runtime) + if (server.auth?.type === 'oauth' || server.oauth_connected) { return 'auth_required'; } - + // Non-OAuth server that's enabled but not yet connected return 'connected_auto'; }; @@ -514,11 +539,15 @@ export function ServersPage() { } setActionLoading(`enable-${server.id}`); + // Optimistically mark as enabled so runtime status events (Connecting/Error) + // are reflected in the UI immediately instead of showing stale "Enable" button + setInstalledServers(prev => prev.map(s => + s.id === server.id ? { ...s, enabled: true } : s + )); try { // Use new ServerManager v2 - handles connection + OAuth in backend await enableServerV2(server.id); - await loadData(); - + // Expand server to show features after connection setTimeout(() => { setExpandedServers(prev => new Set(prev).add(server.id)); @@ -527,6 +556,9 @@ export function ServersPage() { } catch (e) { showToast(String(e), 'error'); } finally { + // Always refresh server list - the backend sets enabled=true in DB before + // attempting connection, so we need to reflect that even on connection failure + await loadData(); setActionLoading(null); } }; @@ -604,12 +636,13 @@ export function ServersPage() { // Only enable if requested (from Enable flow) if (shouldEnable && !server.enabled) { + // Optimistically mark as enabled so runtime status events are reflected + setInstalledServers(prev => prev.map(s => + s.id === serverId ? { ...s, enabled: true, missing_required_inputs: false } : s + )); // Use new ServerManager v2 to enable and connect await enableServerV2(serverId); - - // Reload data AFTER enable to get updated enabled state - await loadData(); - + setTimeout(() => { setExpandedServers(prev => new Set(prev).add(serverId)); loadFeaturesForServer(serverId); @@ -617,16 +650,15 @@ export function ServersPage() { } else if (server.enabled) { // If already enabled, trigger reconnect with new config await retryConnectionV2(serverId); - await loadData(); - } else { - // Just save, don't enable - await loadData(); } - + showToast('Configuration saved', 'success'); } catch (e) { showToast(String(e), 'error'); } finally { + // Always refresh server list to reflect DB state (enabled, config changes) + // even if connection failed + await loadData(); setActionLoading(null); } }; @@ -885,15 +917,13 @@ export function ServersPage() { return (
{/* Server Header */}
-
+
{/* Expand/Collapse button for connected servers */} {isConnected && (
)}
@@ -1015,7 +1050,7 @@ export function ServersPage() { @@ -1067,7 +1102,7 @@ export function ServersPage() { @@ -1089,7 +1124,7 @@ export function ServersPage() { @@ -1124,8 +1159,8 @@ export function ServersPage() { onRefresh={() => handleRefresh(server)} onReconnect={() => handleReconnect(server)} onViewLogs={() => setLogViewerServer({ id: server.id, name: server.name })} + onViewDefinition={() => setDefinitionServer({ id: server.id, name: server.name })} onUninstall={() => handleUninstall(server)} - disabled={!!actionLoading} />
@@ -1583,6 +1618,17 @@ export function ServersPage() { /> )} + {/* Definition Viewer Modal */} + {definitionServer && (() => { + const server = installedServers.find(s => s.id === definitionServer.id); + return server ? ( + setDefinitionServer(null)} + /> + ) : null; + })()} + {/* Config Editor Modal */} {editConfigSpace && ( (null); const prevSpaceId = useRef(null); + // Stable ref for onFeaturesChange to avoid re-subscribing on every render + const onFeaturesChangeRef = useRef(onFeaturesChange); + onFeaturesChangeRef.current = onFeaturesChange; + // Fetch initial statuses const refresh = useCallback(async () => { if (!spaceId) return; @@ -124,8 +128,14 @@ export function useServerManager({ const unsubscribers: Array<() => void> = []; + // Event listeners are async (Tauri listen() returns a Promise). + // Events emitted between the initial getServerStatuses fetch and listener + // activation are lost. Track when all listeners are ready, then re-fetch + // statuses to catch any events missed during the gap. + const listenerPromises: Array void>> = []; + // Status changes - onServerStatus((event: ServerStatusEvent) => { + const statusPromise = onServerStatus((event: ServerStatusEvent) => { if (event.space_id !== spaceId) return; setStatuses((prev) => { @@ -155,30 +165,43 @@ export function useServerManager({ return next; }); } - }).then((unlisten) => unsubscribers.push(unlisten)); + }); + statusPromise.then((unlisten) => unsubscribers.push(unlisten)); + listenerPromises.push(statusPromise); // Auth progress - onAuthProgress((event: AuthProgressEvent) => { + const authPromise = onAuthProgress((event: AuthProgressEvent) => { if (event.space_id !== spaceId) return; setAuthProgress((prev) => ({ ...prev, [event.server_id]: event.remaining_seconds, })); - }).then((unlisten) => unsubscribers.push(unlisten)); + }); + authPromise.then((unlisten) => unsubscribers.push(unlisten)); + listenerPromises.push(authPromise); - // Features updated - if (onFeaturesChange) { - onFeaturesUpdated((event: FeaturesUpdatedEvent) => { + // Features updated (always subscribe, use ref to call latest callback) + const featuresPromise = onFeaturesUpdated( + (event: FeaturesUpdatedEvent) => { if (event.space_id !== spaceId) return; - onFeaturesChange(event); - }).then((unlisten) => unsubscribers.push(unlisten)); - } + onFeaturesChangeRef.current?.(event); + } + ); + featuresPromise.then((unlisten) => unsubscribers.push(unlisten)); + listenerPromises.push(featuresPromise); + + // Once all listeners are active, re-fetch statuses to close the gap + // between the initial fetch and listener activation (startup race fix) + Promise.all(listenerPromises).then(() => { + refresh(); + }); return () => { unsubscribers.forEach((fn) => fn()); }; - }, [spaceId, onFeaturesChange]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [spaceId]); // Actions const enable = useCallback( diff --git a/tests/e2e/specs/registry.spec.ts b/tests/e2e/specs/registry.spec.ts index 54f252b4..97d79f4c 100644 --- a/tests/e2e/specs/registry.spec.ts +++ b/tests/e2e/specs/registry.spec.ts @@ -167,6 +167,29 @@ test.describe('Registry Server Icon Rendering', () => { }); }); +test.describe('Registry Server Detail Modal', () => { + test('should show View JSON button in server detail modal', async ({ page }) => { + const dashboard = new DashboardPage(page); + await dashboard.navigate(); + + await page.locator('nav button:has-text("Discover")').click(); + await page.waitForTimeout(500); + + // Click first server card to open detail modal + const firstCard = page.locator('[data-testid^="server-card-"]').first(); + if (await firstCard.isVisible().catch(() => false)) { + await firstCard.click(); + await page.waitForTimeout(300); + + // The detail modal footer should contain "View JSON" button + await expect(page.locator('.fixed button:has-text("View JSON")')).toBeVisible(); + + // Close modal + await page.keyboard.press('Escape'); + } + }); +}); + test.describe('Registry Filters and Sorting', () => { test('should have filter elements', async ({ page }) => { const dashboard = new DashboardPage(page); diff --git a/tests/e2e/specs/server-lifecycle.wdio.ts b/tests/e2e/specs/server-lifecycle.wdio.ts index d396d685..e149fecf 100644 --- a/tests/e2e/specs/server-lifecycle.wdio.ts +++ b/tests/e2e/specs/server-lifecycle.wdio.ts @@ -106,6 +106,60 @@ describe('Server Installation - GitHub Server (No Inputs)', () => { } }); + it('TC-SL-004: Action menu shows View Logs, View Definition, Uninstall', async () => { + await waitForModalClose(); + // Ensure we're on My Servers page + const myServersButton = await byTestId('nav-my-servers'); + await myServersButton.click(); + await browser.pause(2000); + + // Open the action menu for the GitHub server + const menuButton = await byTestId('action-menu-github-server'); + const isMenuDisplayed = await menuButton.isDisplayed().catch(() => false); + + if (isMenuDisplayed) { + await menuButton.click(); + await browser.pause(500); + + await browser.saveScreenshot('./tests/e2e/screenshots/sl-08-action-menu.png'); + + // Verify View Logs menu item + const viewLogsItem = await byTestId('view-logs-github-server'); + await expect(viewLogsItem).toBeDisplayed(); + + // Verify View Definition menu item + const viewDefItem = await byTestId('view-definition-github-server'); + await expect(viewDefItem).toBeDisplayed(); + + // Verify Uninstall menu item + const uninstallItem = await byTestId('uninstall-menu-github-server'); + await expect(uninstallItem).toBeDisplayed(); + + // Click View Definition to open the definition modal + await viewDefItem.click(); + await browser.pause(1000); + + await browser.saveScreenshot('./tests/e2e/screenshots/sl-09-view-definition.png'); + + // Verify the definition modal is open (contains Monaco editor or JSON content) + const pageSource = await browser.getPageSource(); + const hasDefinitionModal = + pageSource.includes('Definition') || + pageSource.includes('monaco') || + pageSource.includes('GitHub'); + + expect(hasDefinitionModal).toBe(true); + + // Close the modal + await browser.keys('Escape'); + await browser.pause(500); + } else { + // Server card may not be present (install may have failed) + const pageSource = await browser.getPageSource(); + expect(pageSource.includes('GitHub') || pageSource.includes('My Servers')).toBe(true); + } + }); + it('TC-SD-005: Uninstall GitHub Server', async () => { await waitForModalClose(); const discoverButton = await byTestId('nav-discover'); diff --git a/tests/e2e/specs/servers.spec.ts b/tests/e2e/specs/servers.spec.ts index 5a570574..fd221fab 100644 --- a/tests/e2e/specs/servers.spec.ts +++ b/tests/e2e/specs/servers.spec.ts @@ -90,6 +90,32 @@ test.describe('Server Actions', () => { }); }); +test.describe('Server Action Menu', () => { + test('should show View Logs and View Definition in action menu', async ({ page }) => { + const dashboard = new DashboardPage(page); + await dashboard.navigate(); + + await page.locator('nav button:has-text("My Servers")').click(); + + // Find any server card with a menu button (three-dot / MoreVertical) + const menuButtons = page.locator('button[aria-label="More actions"]'); + const count = await menuButtons.count(); + + if (count > 0) { + await menuButtons.first().click(); + + // The menu should contain View Logs and View Definition items + await expect(page.getByRole('menuitem', { name: /View Logs/i })).toBeVisible(); + await expect(page.getByRole('menuitem', { name: /View Definition/i })).toBeVisible(); + await expect(page.getByRole('menuitem', { name: /Uninstall/i })).toBeVisible(); + + // Close menu + await page.keyboard.press('Escape'); + } + // If no servers installed, test passes silently + }); +}); + test.describe('Server Toast Notifications', () => { // Skip in web mode - requires Tauri API for server enable/disable test.skip('should show success toast on server enable', async ({ page }) => { diff --git a/tests/ts/components/ServerDetailModal.test.tsx b/tests/ts/components/ServerDetailModal.test.tsx index e1585c3a..5ef44267 100644 --- a/tests/ts/components/ServerDetailModal.test.tsx +++ b/tests/ts/components/ServerDetailModal.test.tsx @@ -106,4 +106,10 @@ describe('ServerDetailModal', () => { render(); expect(screen.getByText('Uninstall')).toBeInTheDocument(); }); + + it('should render View JSON button in footer', () => { + const server = makeServer(); + render(); + expect(screen.getByText('View JSON')).toBeInTheDocument(); + }); });