mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-17 00:40:14 +01:00
🧩 refactor: Decouple MCP Config from Startup Config (#10689)
* Decouple mcp config from start up config * Chore: Work on AI Review and Copilot Comments - setRawConfig is not needed since the private raw config is not needed any more - !!serversLoading bug fixed - added unit tests for route /api/mcp/servers - copilot comments addressed * chore: remove comments * chore: rename data-provider dir for MCP * chore: reorganize mcp specific query hooks * fix: consolidate imports for MCP server manager * chore: add dev-staging branch to frontend review workflow triggers * feat: add GitHub Actions workflow for building and pushing Docker images to GitHub Container Registry and Docker Hub * fix: update label for tag input in BookmarkForm tests to improve clarity --------- Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com> Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
Atef Bellaaj
Danny Avila
parent
98b188f26c
commit
ef1b7f0157
36 changed files with 548 additions and 301 deletions
|
|
@ -6,7 +6,7 @@ import { Constants, LocalStorageKeys } from 'librechat-data-provider';
|
|||
import { ephemeralAgentByConvoId } from '~/store';
|
||||
import { setTimestamp } from '~/utils/timestamps';
|
||||
import { useMCPSelect } from '../useMCPSelect';
|
||||
import * as dataProvider from '~/data-provider';
|
||||
import { MCPServerDefinition } from '../useMCPServerManager';
|
||||
|
||||
// Mock dependencies
|
||||
jest.mock('~/utils/timestamps', () => ({
|
||||
|
|
@ -15,27 +15,28 @@ jest.mock('~/utils/timestamps', () => ({
|
|||
|
||||
jest.mock('lodash/isEqual', () => jest.fn((a, b) => JSON.stringify(a) === JSON.stringify(b)));
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
...jest.requireActual('~/data-provider'),
|
||||
useGetStartupConfig: jest.fn(),
|
||||
}));
|
||||
// Helper to create MCPServerDefinition objects
|
||||
const createMCPServers = (serverNames: string[]): MCPServerDefinition[] => {
|
||||
return serverNames.map((serverName) => ({
|
||||
serverName,
|
||||
config: {
|
||||
url: 'http://mcp',
|
||||
},
|
||||
effectivePermissions: 15, // All permissions (VIEW=1, EDIT=2, DELETE=4, SHARE=8)
|
||||
}));
|
||||
};
|
||||
|
||||
const createWrapper = (mcpServers: string[] = []) => {
|
||||
// Create a new Jotai store for each test to ensure clean state
|
||||
const store = createStore();
|
||||
|
||||
// Mock the startup config
|
||||
(dataProvider.useGetStartupConfig as jest.Mock).mockReturnValue({
|
||||
data: { mcpServers: Object.fromEntries(mcpServers.map((v) => [v, {}])) },
|
||||
isLoading: false,
|
||||
});
|
||||
const servers = createMCPServers(mcpServers);
|
||||
|
||||
const Wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => (
|
||||
<RecoilRoot>
|
||||
<Provider store={store}>{children}</Provider>
|
||||
</RecoilRoot>
|
||||
);
|
||||
return Wrapper;
|
||||
return { Wrapper, servers };
|
||||
};
|
||||
|
||||
describe('useMCPSelect', () => {
|
||||
|
|
@ -46,8 +47,9 @@ describe('useMCPSelect', () => {
|
|||
|
||||
describe('Basic Functionality', () => {
|
||||
it('should initialize with default values', () => {
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
expect(result.current.mcpValues).toEqual([]);
|
||||
|
|
@ -58,16 +60,18 @@ describe('useMCPSelect', () => {
|
|||
|
||||
it('should use conversationId when provided', () => {
|
||||
const conversationId = 'test-convo-123';
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId }), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId, servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
expect(result.current.mcpValues).toEqual([]);
|
||||
});
|
||||
|
||||
it('should use NEW_CONVO constant when conversationId is null', () => {
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId: null }), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId: null, servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
expect(result.current.mcpValues).toEqual([]);
|
||||
|
|
@ -76,8 +80,9 @@ describe('useMCPSelect', () => {
|
|||
|
||||
describe('State Updates', () => {
|
||||
it('should update mcpValues when setMCPValues is called', async () => {
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(['value1', 'value2']),
|
||||
const { Wrapper, servers } = createWrapper(['value1', 'value2']);
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
const newValues = ['value1', 'value2'];
|
||||
|
|
@ -92,8 +97,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should not update mcpValues if non-array is passed', () => {
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
act(() => {
|
||||
|
|
@ -105,8 +111,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should update isPinned state', () => {
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
// Default is true
|
||||
|
|
@ -131,8 +138,9 @@ describe('useMCPSelect', () => {
|
|||
describe('Timestamp Management', () => {
|
||||
it('should set timestamp when mcpValues is updated with values', async () => {
|
||||
const conversationId = 'test-convo';
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId }), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId, servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
const newValues = ['value1', 'value2'];
|
||||
|
|
@ -148,8 +156,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should not set timestamp when mcpValues is empty', async () => {
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
act(() => {
|
||||
|
|
@ -164,8 +173,9 @@ describe('useMCPSelect', () => {
|
|||
|
||||
describe('Race Conditions and Infinite Loops Prevention', () => {
|
||||
it('should not create infinite loop when syncing between Jotai and Recoil states', async () => {
|
||||
const { result, rerender } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result, rerender } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
let renderCount = 0;
|
||||
|
|
@ -196,8 +206,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should handle rapid consecutive updates without race conditions', async () => {
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
const updates = [
|
||||
|
|
@ -222,8 +233,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should maintain stable setter function reference', () => {
|
||||
const { result, rerender } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result, rerender } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
const firstSetMCPValues = result.current.setMCPValues;
|
||||
|
|
@ -238,10 +250,11 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should handle switching conversation IDs without issues', async () => {
|
||||
const { Wrapper, servers } = createWrapper(['convo1-value', 'convo2-value']);
|
||||
const { result, rerender } = renderHook(
|
||||
({ conversationId }) => useMCPSelect({ conversationId }),
|
||||
({ conversationId }) => useMCPSelect({ conversationId, servers }),
|
||||
{
|
||||
wrapper: createWrapper(['convo1-value', 'convo2-value']),
|
||||
wrapper: Wrapper,
|
||||
initialProps: { conversationId: 'convo1' },
|
||||
},
|
||||
);
|
||||
|
|
@ -283,18 +296,18 @@ describe('useMCPSelect', () => {
|
|||
describe('Ephemeral Agent Synchronization', () => {
|
||||
it('should sync mcpValues when ephemeralAgent is updated externally', async () => {
|
||||
// Create a shared wrapper for both hooks to share the same Recoil/Jotai context
|
||||
const wrapper = createWrapper(['external-value1', 'external-value2']);
|
||||
const { Wrapper, servers } = createWrapper(['external-value1', 'external-value2']);
|
||||
|
||||
// Create a component that uses both hooks to ensure they share state
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const [ephemeralAgent, setEphemeralAgent] = useRecoilState(
|
||||
ephemeralAgentByConvoId(Constants.NEW_CONVO),
|
||||
);
|
||||
return { mcpHook, ephemeralAgent, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
// Simulate external update to ephemeralAgent (e.g., from another component)
|
||||
const externalMcpValues = ['external-value1', 'external-value2'];
|
||||
|
|
@ -311,15 +324,15 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should filter out MCPs not in configured servers', async () => {
|
||||
const wrapper = createWrapper(['server1', 'server2']);
|
||||
const { Wrapper, servers } = createWrapper(['server1', 'server2']);
|
||||
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
act(() => {
|
||||
result.current.setEphemeralAgent({
|
||||
|
|
@ -333,15 +346,15 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should clear all MCPs when none are in configured servers', async () => {
|
||||
const wrapper = createWrapper(['server1', 'server2']);
|
||||
const { Wrapper, servers } = createWrapper(['server1', 'server2']);
|
||||
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
act(() => {
|
||||
result.current.setEphemeralAgent({
|
||||
|
|
@ -355,15 +368,15 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should keep all MCPs when all are in configured servers', async () => {
|
||||
const wrapper = createWrapper(['server1', 'server2', 'server3']);
|
||||
const { Wrapper, servers } = createWrapper(['server1', 'server2', 'server3']);
|
||||
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
act(() => {
|
||||
result.current.setEphemeralAgent({
|
||||
|
|
@ -378,16 +391,16 @@ describe('useMCPSelect', () => {
|
|||
|
||||
it('should update ephemeralAgent when mcpValues changes through hook', async () => {
|
||||
// Create a shared wrapper for both hooks
|
||||
const wrapper = createWrapper(['hook-value1', 'hook-value2']);
|
||||
const { Wrapper, servers } = createWrapper(['hook-value1', 'hook-value2']);
|
||||
|
||||
// Create a component that uses both the hook and accesses Recoil state
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const ephemeralAgent = useRecoilValue(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, ephemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
const newValues = ['hook-value1', 'hook-value2'];
|
||||
|
||||
|
|
@ -404,16 +417,16 @@ describe('useMCPSelect', () => {
|
|||
|
||||
it('should handle empty ephemeralAgent.mcp array correctly', async () => {
|
||||
// Create a shared wrapper
|
||||
const wrapper = createWrapper(['initial-value']);
|
||||
const { Wrapper, servers } = createWrapper(['initial-value']);
|
||||
|
||||
// Create a component that uses both hooks
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
// Set initial values
|
||||
act(() => {
|
||||
|
|
@ -438,16 +451,16 @@ describe('useMCPSelect', () => {
|
|||
|
||||
it('should handle ephemeralAgent with clear mcp value', async () => {
|
||||
// Create a shared wrapper
|
||||
const wrapper = createWrapper(['server1', 'server2']);
|
||||
const { Wrapper, servers } = createWrapper(['server1', 'server2']);
|
||||
|
||||
// Create a component that uses both hooks
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
// Set initial values
|
||||
act(() => {
|
||||
|
|
@ -473,15 +486,21 @@ describe('useMCPSelect', () => {
|
|||
|
||||
it('should properly sync non-empty arrays from ephemeralAgent', async () => {
|
||||
// Additional test to ensure non-empty arrays DO sync
|
||||
const wrapper = createWrapper(['value1', 'value2', 'value3', 'value4', 'value5']);
|
||||
const { Wrapper, servers } = createWrapper([
|
||||
'value1',
|
||||
'value2',
|
||||
'value3',
|
||||
'value4',
|
||||
'value5',
|
||||
]);
|
||||
|
||||
const TestComponent = () => {
|
||||
const mcpHook = useMCPSelect({});
|
||||
const mcpHook = useMCPSelect({ servers });
|
||||
const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(Constants.NEW_CONVO));
|
||||
return { mcpHook, setEphemeralAgent };
|
||||
};
|
||||
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper });
|
||||
const { result } = renderHook(() => TestComponent(), { wrapper: Wrapper });
|
||||
|
||||
// Set initial values through ephemeralAgent with non-empty array
|
||||
const initialValues = ['value1', 'value2'];
|
||||
|
|
@ -513,8 +532,9 @@ describe('useMCPSelect', () => {
|
|||
|
||||
describe('Edge Cases', () => {
|
||||
it('should handle undefined conversationId', () => {
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId: undefined }), {
|
||||
wrapper: createWrapper(['test']),
|
||||
const { Wrapper, servers } = createWrapper(['test']);
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId: undefined, servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
expect(result.current.mcpValues).toEqual([]);
|
||||
|
|
@ -527,8 +547,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should handle empty string conversationId', () => {
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId: '' }), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result } = renderHook(() => useMCPSelect({ conversationId: '', servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
expect(result.current.mcpValues).toEqual([]);
|
||||
|
|
@ -536,8 +557,9 @@ describe('useMCPSelect', () => {
|
|||
|
||||
it('should handle very large arrays without performance issues', async () => {
|
||||
const largeArray = Array.from({ length: 1000 }, (_, i) => `value-${i}`);
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(largeArray),
|
||||
const { Wrapper, servers } = createWrapper(largeArray);
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
const startTime = performance.now();
|
||||
|
|
@ -558,8 +580,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should cleanup properly on unmount', () => {
|
||||
const { unmount } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { unmount } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
// Should unmount without errors
|
||||
|
|
@ -570,8 +593,9 @@ describe('useMCPSelect', () => {
|
|||
describe('Memory Leak Prevention', () => {
|
||||
it('should not leak memory on repeated updates', async () => {
|
||||
const values = Array.from({ length: 100 }, (_, i) => `value-${i}`);
|
||||
const { result } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(values),
|
||||
const { Wrapper, servers } = createWrapper(values);
|
||||
const { result } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
// Perform many updates to test for memory leaks
|
||||
|
|
@ -586,8 +610,9 @@ describe('useMCPSelect', () => {
|
|||
});
|
||||
|
||||
it('should handle component remounting', () => {
|
||||
const { result, unmount } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper, servers } = createWrapper();
|
||||
const { result, unmount } = renderHook(() => useMCPSelect({ servers }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
act(() => {
|
||||
|
|
@ -597,8 +622,9 @@ describe('useMCPSelect', () => {
|
|||
unmount();
|
||||
|
||||
// Remount
|
||||
const { result: newResult } = renderHook(() => useMCPSelect({}), {
|
||||
wrapper: createWrapper(),
|
||||
const { Wrapper: Wrapper2, servers: servers2 } = createWrapper();
|
||||
const { result: newResult } = renderHook(() => useMCPSelect({ servers: servers2 }), {
|
||||
wrapper: Wrapper2,
|
||||
});
|
||||
|
||||
// Should handle remounting gracefully
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue