fix(extensions): harden mcp diagnostics runtime guards
This commit is contained in:
parent
c17a08c2d3
commit
f8c11af5b9
2 changed files with 105 additions and 5 deletions
|
|
@ -100,6 +100,7 @@ export const McpServersPanel = ({
|
|||
mcpDiagnosticsLastCheckedAtFallback,
|
||||
runMcpDiagnostics,
|
||||
cliStatus,
|
||||
cliStatusLoading,
|
||||
} = useStore(
|
||||
useShallow((s) => ({
|
||||
browseCatalog: s.mcpBrowseCatalog,
|
||||
|
|
@ -120,6 +121,7 @@ export const McpServersPanel = ({
|
|||
mcpDiagnosticsLastCheckedAtFallback: s.mcpDiagnosticsLastCheckedAt,
|
||||
runMcpDiagnostics: s.runMcpDiagnostics,
|
||||
cliStatus: s.cliStatus,
|
||||
cliStatusLoading: s.cliStatusLoading,
|
||||
}))
|
||||
);
|
||||
const installedServers =
|
||||
|
|
@ -144,9 +146,27 @@ export const McpServersPanel = ({
|
|||
}
|
||||
}, [browseCatalog.length, browseError, browseLoading, mcpBrowse]);
|
||||
|
||||
const diagnosticsDisableReason = useMemo(() => {
|
||||
if (cliStatusLoading) {
|
||||
return 'Checking runtime status...';
|
||||
}
|
||||
|
||||
if (cliStatus?.installed === false) {
|
||||
if (cliStatus.binaryPath && cliStatus.launchError) {
|
||||
return 'The configured runtime was found but failed to start. Open the Dashboard to repair or reinstall it.';
|
||||
}
|
||||
return 'The configured runtime is required. Install or repair it from the Dashboard.';
|
||||
}
|
||||
|
||||
return null;
|
||||
}, [cliStatus, cliStatusLoading]);
|
||||
|
||||
useEffect(() => {
|
||||
if (diagnosticsDisableReason) {
|
||||
return;
|
||||
}
|
||||
void runMcpDiagnostics(projectPath ?? undefined);
|
||||
}, [projectPath, runMcpDiagnostics]);
|
||||
}, [diagnosticsDisableReason, projectPath, runMcpDiagnostics]);
|
||||
|
||||
// Fetch GitHub stars after catalog loads (fire-and-forget)
|
||||
useEffect(() => {
|
||||
|
|
@ -240,6 +260,8 @@ export const McpServersPanel = ({
|
|||
<p className="text-xs text-text-muted">
|
||||
{mcpDiagnosticsLoading ? (
|
||||
<>Checking installed MCP servers via {runtimeLabel} ...</>
|
||||
) : diagnosticsDisableReason ? (
|
||||
diagnosticsDisableReason
|
||||
) : mcpDiagnosticsLastCheckedAt ? (
|
||||
`Last checked ${formatRelativeTime(new Date(mcpDiagnosticsLastCheckedAt).toISOString())}`
|
||||
) : (
|
||||
|
|
@ -251,7 +273,7 @@ export const McpServersPanel = ({
|
|||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() => void runMcpDiagnostics(projectPath ?? undefined)}
|
||||
disabled={mcpDiagnosticsLoading}
|
||||
disabled={mcpDiagnosticsLoading || Boolean(diagnosticsDisableReason)}
|
||||
className="whitespace-nowrap"
|
||||
>
|
||||
<RefreshCw
|
||||
|
|
@ -299,9 +321,7 @@ export const McpServersPanel = ({
|
|||
))}
|
||||
</div>
|
||||
) : (
|
||||
<p className="text-xs text-text-muted">
|
||||
Waiting for <code>{diagnosticsCommand}</code> results...
|
||||
</p>
|
||||
<p className="text-xs text-text-muted">Waiting for diagnostics results...</p>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -34,8 +34,12 @@ interface StoreState {
|
|||
mcpDiagnosticsLastCheckedAt: number | null;
|
||||
mcpDiagnosticsLastCheckedAtByProjectPath?: Record<string, number | null>;
|
||||
runMcpDiagnostics: ReturnType<typeof vi.fn>;
|
||||
cliStatusLoading: boolean;
|
||||
cliStatus?: {
|
||||
flavor?: 'claude' | 'agent_teams_orchestrator';
|
||||
installed?: boolean;
|
||||
binaryPath?: string | null;
|
||||
launchError?: string | null;
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -144,6 +148,7 @@ describe('McpServersPanel initial browse loading', () => {
|
|||
storeState.mcpDiagnosticsLastCheckedAt = null;
|
||||
storeState.mcpDiagnosticsLastCheckedAtByProjectPath = undefined;
|
||||
storeState.runMcpDiagnostics = vi.fn();
|
||||
storeState.cliStatusLoading = false;
|
||||
storeState.cliStatus = undefined;
|
||||
});
|
||||
|
||||
|
|
@ -309,4 +314,79 @@ describe('McpServersPanel initial browse loading', () => {
|
|||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
|
||||
it('does not auto-run diagnostics when the configured runtime is unavailable', async () => {
|
||||
storeState.cliStatus = {
|
||||
flavor: 'agent_teams_orchestrator',
|
||||
installed: false,
|
||||
binaryPath: null,
|
||||
launchError: null,
|
||||
};
|
||||
|
||||
const host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
const root = createRoot(host);
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(McpServersPanel, {
|
||||
projectPath: null,
|
||||
mcpSearchQuery: '',
|
||||
mcpSearch: vi.fn(),
|
||||
mcpSearchResults: [],
|
||||
mcpSearchLoading: false,
|
||||
mcpSearchWarnings: [],
|
||||
selectedMcpServerId: null,
|
||||
setSelectedMcpServerId: vi.fn(),
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(storeState.runMcpDiagnostics).not.toHaveBeenCalled();
|
||||
expect(host.textContent).toContain(
|
||||
'The configured runtime is required. Install or repair it from the Dashboard.'
|
||||
);
|
||||
const checkStatusButton = Array.from(host.querySelectorAll('button')).find((button) =>
|
||||
button.textContent?.includes('Check Status')
|
||||
);
|
||||
expect(checkStatusButton).toBeDefined();
|
||||
expect((checkStatusButton as HTMLButtonElement).disabled).toBe(true);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
|
||||
it('renders provider-neutral waiting copy while diagnostics are still running', async () => {
|
||||
storeState.mcpDiagnosticsLoading = true;
|
||||
|
||||
const host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
const root = createRoot(host);
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(McpServersPanel, {
|
||||
projectPath: null,
|
||||
mcpSearchQuery: '',
|
||||
mcpSearch: vi.fn(),
|
||||
mcpSearchResults: [],
|
||||
mcpSearchLoading: false,
|
||||
mcpSearchWarnings: [],
|
||||
selectedMcpServerId: null,
|
||||
setSelectedMcpServerId: vi.fn(),
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(host.textContent).toContain('Waiting for diagnostics results...');
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue