diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index adeb9f7ca99..d6244dfe0ab 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -451,10 +451,11 @@ const loadTools = async ({ /** MCP server tools are initialized sequentially by server */ let index = -1; const failedMCPServers = new Set(); - const safeUser = createSafeUser(options.req?.user); + const safeUser = createSafeUser(options.req?.user, user); const requestScopedConnections = options.requestScopedConnections ?? getMCPRequestContext(options.req, options.res); + for (const [serverName, toolConfigs] of Object.entries(requestedMCPTools)) { index++; /** @type {LCAvailableTools} */ diff --git a/api/app/clients/tools/util/handleTools.test.js b/api/app/clients/tools/util/handleTools.test.js index 697649e3bde..9870650772f 100644 --- a/api/app/clients/tools/util/handleTools.test.js +++ b/api/app/clients/tools/util/handleTools.test.js @@ -451,4 +451,52 @@ describe('Tool Handlers', () => { ); }); }); + + describe('loadTools MCP user id propagation', () => { + // Regression guard: MCP tool-call requests shipped the literal "{{LIBRECHAT_USER_ID}}" + // placeholder in x-user-id headers because `loadTools` built the MCP `user` object from + // `options.req?.user` alone. When `req.user` is a plain object deserialized from the + // passport session (no `id` virtual, no `_id`), that object resolves to no usable id even + // though `loadTools` is already given the caller's resolved id via its own `user` param + // (see ToolService.js: `loadTools({ user: req.user.id, ... })`). Assert that id makes it + // into the object handed to `createMCPTools`. + const mcpServerName = 'test-mcp-server'; + const mcpAllToolName = `${Constants.mcp_all}${Constants.mcp_delimiter}${mcpServerName}`; + + beforeEach(() => { + mockGetServerConfig.mockResolvedValue({ startup: true }); + mockCreateMCPTools.mockResolvedValue([]); + }); + + it('falls back to the loadTools `user` param when req.user has neither id nor _id', async () => { + const userId = fakeUser._id.toString(); + const sessionUser = { email: 'fakeuser@example.com', provider: 'local' }; + + await loadTools({ + user: userId, + tools: [mcpAllToolName], + options: { req: { user: sessionUser } }, + }); + + expect(mockCreateMCPTools).toHaveBeenCalledWith( + expect.objectContaining({ + user: expect.objectContaining({ id: userId }), + }), + ); + }); + + it('prefers a resolvable req.user id over the loadTools `user` param', async () => { + await loadTools({ + user: fakeUser._id.toString(), + tools: [mcpAllToolName], + options: { req: { user: { id: 'explicit-req-user-id' } } }, + }); + + expect(mockCreateMCPTools).toHaveBeenCalledWith( + expect.objectContaining({ + user: expect.objectContaining({ id: 'explicit-req-user-id' }), + }), + ); + }); + }); }); diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index fda2bdd6167..7f9000c48a7 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -1345,6 +1345,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => { 'author', 'avatar', 'category', + 'conversation_starters', 'description', 'id', 'is_promoted', diff --git a/packages/api/src/mcp/__tests__/mcp.spec.ts b/packages/api/src/mcp/__tests__/mcp.spec.ts index 539c04f6b94..ef6d2a1a497 100644 --- a/packages/api/src/mcp/__tests__/mcp.spec.ts +++ b/packages/api/src/mcp/__tests__/mcp.spec.ts @@ -586,7 +586,9 @@ describe('Environment Variable Extraction (MCP)', () => { const result1 = processMCPEnv({ options: obj1, user: userWithId }); expect('headers' in result1 && result1.headers?.['User-Id']).toBe('user-123'); - // Test with '_id' property only (should not work since we only check 'id') + // Test with '_id' property only. Our fork accepts _id as a fallback for id + // (PRs #29/#30/#31/#32 threaded this through createSafeUser and loadTools) + // because passport-deserialized session user objects only carry _id. const userWithUnderscore = createTestUser({ id: undefined, // Remove default id to test _id _id: 'user-456', @@ -601,8 +603,7 @@ describe('Environment Variable Extraction (MCP)', () => { }; const result2 = processMCPEnv({ options: obj2, user: userWithUnderscore }); - // Since we don't check _id, the placeholder should remain unchanged - expect('headers' in result2 && result2.headers?.['User-Id']).toBe('{{LIBRECHAT_USER_ID}}'); + expect('headers' in result2 && result2.headers?.['User-Id']).toBe('user-456'); // Test with both properties (id takes precedence) const userWithBoth = createTestUser({