From cb41ba14b244b53a7204575d11b06fd1cac30722 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 1 Apr 2026 21:13:07 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=81=20fix:=20Pass=20recursionLimit=20t?= =?UTF-8?q?o=20OpenAI-Compatible=20Agents=20API=20Endpoint=20(#12510)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: pass recursionLimit to processStream in OpenAI-compatible agents API The OpenAI-compatible endpoint never passed recursionLimit to LangGraph's processStream(), silently capping all API-based agent calls at the default 25 steps. Mirror the 3-step cascade already used by the UI path (client.js): yaml config default → per-agent DB override → max cap. * refactor: extract resolveRecursionLimit into shared utility Extract the 3-step recursion limit cascade into a shared resolveRecursionLimit() function in @librechat/api. Both openai.js and client.js now call this single source of truth. Also fixes falsy-guard edge cases where recursion_limit=0 or maxRecursionLimit=0 would silently misbehave, by using explicit typeof + positive checks. Includes unit tests covering all cascade branches and edge cases. * refactor: use resolveRecursionLimit in openai.js and client.js Replace duplicated cascade logic in both controllers with the shared resolveRecursionLimit() utility from @librechat/api. In openai.js: hoist agentsEConfig to avoid double property walk, remove displaced comment, add integration test assertions. In client.js: remove inline cascade that was overriding config after initial assignment. * fix: hoist processStream mock for test accessibility The processStream mock was created inline inside mockResolvedValue, making it inaccessible via createRun.mock.results (which returns the Promise, not the resolved value). Hoist it to a module-level variable so tests can assert on it directly. * test: improve test isolation and boundary coverage Use mockReturnValueOnce instead of mockReturnValue to prevent mock leaking across test boundaries. Add boundary tests for downward agent override and exact-match maxRecursionLimit. --- .../agents/__tests__/openai.spec.js | 36 ++++++++++- api/server/controllers/agents/client.js | 14 +---- api/server/controllers/agents/openai.js | 9 ++- packages/api/src/agents/config.spec.ts | 62 +++++++++++++++++++ packages/api/src/agents/config.ts | 30 +++++++++ packages/api/src/agents/index.ts | 1 + 6 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 packages/api/src/agents/config.spec.ts create mode 100644 packages/api/src/agents/config.ts diff --git a/api/server/controllers/agents/__tests__/openai.spec.js b/api/server/controllers/agents/__tests__/openai.spec.js index c2f13f7837..c959be6cf4 100644 --- a/api/server/controllers/agents/__tests__/openai.spec.js +++ b/api/server/controllers/agents/__tests__/openai.spec.js @@ -3,6 +3,7 @@ * Tests that recordCollectedUsage is called correctly for token spending */ +const mockProcessStream = jest.fn().mockResolvedValue(undefined); const mockSpendTokens = jest.fn().mockResolvedValue({}); const mockSpendStructuredTokens = jest.fn().mockResolvedValue({}); const mockRecordCollectedUsage = jest @@ -35,7 +36,7 @@ jest.mock('@librechat/agents', () => ({ jest.mock('@librechat/api', () => ({ writeSSE: jest.fn(), createRun: jest.fn().mockResolvedValue({ - processStream: jest.fn().mockResolvedValue(undefined), + processStream: mockProcessStream, }), createChunk: jest.fn().mockReturnValue({}), buildToolSet: jest.fn().mockReturnValue(new Set()), @@ -68,6 +69,7 @@ jest.mock('@librechat/api', () => ({ toolCalls: new Map(), usage: { promptTokens: 100, completionTokens: 50, reasoningTokens: 0 }, }), + resolveRecursionLimit: jest.fn().mockReturnValue(50), createToolExecuteHandler: jest.fn().mockReturnValue({ handle: jest.fn() }), isChatCompletionValidationFailure: jest.fn().mockReturnValue(false), })); @@ -286,4 +288,36 @@ describe('OpenAIChatCompletionController', () => { ); }); }); + + describe('recursionLimit resolution', () => { + it('should pass resolveRecursionLimit result to processStream config', async () => { + const { resolveRecursionLimit } = require('@librechat/api'); + resolveRecursionLimit.mockReturnValueOnce(75); + + await OpenAIChatCompletionController(req, res); + + expect(mockProcessStream).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ recursionLimit: 75 }), + expect.anything(), + ); + }); + + it('should call resolveRecursionLimit with agentsEConfig and agent', async () => { + const { resolveRecursionLimit } = require('@librechat/api'); + const { getAgent } = require('~/models'); + const mockAgent = { id: 'agent-123', name: 'Test', recursion_limit: 200 }; + getAgent.mockResolvedValueOnce(mockAgent); + + req.config = { + endpoints: { + agents: { recursionLimit: 100, maxRecursionLimit: 150, allowedProviders: [] }, + }, + }; + + await OpenAIChatCompletionController(req, res); + + expect(resolveRecursionLimit).toHaveBeenCalledWith(req.config.endpoints.agents, mockAgent); + }); + }); }); diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index d6795a4be9..3c1f91bd60 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -21,6 +21,7 @@ const { recordCollectedUsage, GenerationJobManager, getTransactionsConfig, + resolveRecursionLimit, createMemoryProcessor, loadAgent: loadAgentFn, createMultiAgentMapper, @@ -733,7 +734,7 @@ class AgentClient extends BaseClient { }, user: createSafeUser(this.options.req.user), }, - recursionLimit: agentsEConfig?.recursionLimit ?? 50, + recursionLimit: resolveRecursionLimit(agentsEConfig, this.options.agent), signal: abortController.signal, streamMode: 'values', version: 'v2', @@ -781,17 +782,6 @@ class AgentClient extends BaseClient { agents.push(...this.agentConfigs.values()); } - if (agents[0].recursion_limit && typeof agents[0].recursion_limit === 'number') { - config.recursionLimit = agents[0].recursion_limit; - } - - if ( - agentsEConfig?.maxRecursionLimit && - config.recursionLimit > agentsEConfig?.maxRecursionLimit - ) { - config.recursionLimit = agentsEConfig?.maxRecursionLimit; - } - // TODO: needs to be added as part of AgentContext initialization // const noSystemModelRegex = [/\b(o1-preview|o1-mini|amazon\.titan-text)\b/gi]; // const noSystemMessages = noSystemModelRegex.some((regex) => diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index b649058806..9fa3af82c3 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -15,6 +15,7 @@ const { createErrorResponse, recordCollectedUsage, getTransactionsConfig, + resolveRecursionLimit, createToolExecuteHandler, buildNonStreamingResponse, createOpenAIStreamTracker, @@ -194,10 +195,8 @@ const OpenAIChatCompletionController = async (req, res) => { const conversationId = request.conversation_id ?? nanoid(); const parentMessageId = request.parent_message_id ?? null; - // Build allowed providers set - const allowedProviders = new Set( - appConfig?.endpoints?.[EModelEndpoint.agents]?.allowedProviders, - ); + const agentsEConfig = appConfig?.endpoints?.[EModelEndpoint.agents]; + const allowedProviders = new Set(agentsEConfig?.allowedProviders); // Create tool loader const loadTools = createToolLoader(abortController.signal); @@ -491,7 +490,6 @@ const OpenAIChatCompletionController = async (req, res) => { throw new Error('Failed to create agent run'); } - // Process the stream const config = { runName: 'AgentRun', configurable: { @@ -504,6 +502,7 @@ const OpenAIChatCompletionController = async (req, res) => { }, ...(userMCPAuthMap != null && { userMCPAuthMap }), }, + recursionLimit: resolveRecursionLimit(agentsEConfig, agent), signal: abortController.signal, streamMode: 'values', version: 'v2', diff --git a/packages/api/src/agents/config.spec.ts b/packages/api/src/agents/config.spec.ts new file mode 100644 index 0000000000..d09282b5e2 --- /dev/null +++ b/packages/api/src/agents/config.spec.ts @@ -0,0 +1,62 @@ +import type { TAgentsEndpoint } from 'librechat-data-provider'; +import { resolveRecursionLimit } from './config'; + +describe('resolveRecursionLimit', () => { + it('returns default 50 when no config or agent provided', () => { + expect(resolveRecursionLimit(undefined, undefined)).toBe(50); + }); + + it('returns default 50 when config has no recursionLimit', () => { + expect(resolveRecursionLimit({} as TAgentsEndpoint, {})).toBe(50); + }); + + it('uses yaml recursionLimit when set', () => { + const config = { recursionLimit: 100 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, {})).toBe(100); + }); + + it('overrides with agent.recursion_limit when set', () => { + const config = { recursionLimit: 100 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 200 })).toBe(200); + }); + + it('caps at maxRecursionLimit', () => { + const config = { recursionLimit: 100, maxRecursionLimit: 150 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 200 })).toBe(150); + }); + + it('caps yaml default at maxRecursionLimit', () => { + const config = { recursionLimit: 200, maxRecursionLimit: 100 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, {})).toBe(100); + }); + + it('ignores agent.recursion_limit of 0', () => { + const config = { recursionLimit: 100 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 0 })).toBe(100); + }); + + it('ignores negative agent.recursion_limit', () => { + const config = { recursionLimit: 100 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: -5 })).toBe(100); + }); + + it('ignores maxRecursionLimit of 0', () => { + const config = { recursionLimit: 100, maxRecursionLimit: 0 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 200 })).toBe(200); + }); + + it('does not cap when recursionLimit is within maxRecursionLimit', () => { + const config = { recursionLimit: 50, maxRecursionLimit: 200 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 150 })).toBe(150); + }); + + it('allows agent to override downward below yaml default', () => { + const config = { recursionLimit: 100 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 30 })).toBe(30); + }); + + it('does not cap when agent.recursion_limit equals maxRecursionLimit', () => { + const config = { recursionLimit: 50, maxRecursionLimit: 150 } as TAgentsEndpoint; + expect(resolveRecursionLimit(config, { recursion_limit: 150 })).toBe(150); + }); +}); diff --git a/packages/api/src/agents/config.ts b/packages/api/src/agents/config.ts new file mode 100644 index 0000000000..c5c1808f66 --- /dev/null +++ b/packages/api/src/agents/config.ts @@ -0,0 +1,30 @@ +import type { TAgentsEndpoint } from 'librechat-data-provider'; + +const DEFAULT_RECURSION_LIMIT = 50; + +/** + * Resolves the effective recursion limit for an agent run via a 3-step cascade: + * 1. YAML endpoint config default (falls back to 50) + * 2. Per-agent DB override (if set and positive) + * 3. Global max cap from YAML (if set and positive) + */ +export function resolveRecursionLimit( + agentsEConfig: TAgentsEndpoint | undefined, + agent: { recursion_limit?: number } | undefined, +): number { + let limit = agentsEConfig?.recursionLimit ?? DEFAULT_RECURSION_LIMIT; + + if (typeof agent?.recursion_limit === 'number' && agent.recursion_limit > 0) { + limit = agent.recursion_limit; + } + + if ( + typeof agentsEConfig?.maxRecursionLimit === 'number' && + agentsEConfig.maxRecursionLimit > 0 && + limit > agentsEConfig.maxRecursionLimit + ) { + limit = agentsEConfig.maxRecursionLimit; + } + + return limit; +} diff --git a/packages/api/src/agents/index.ts b/packages/api/src/agents/index.ts index ffb8cec332..53f7f60a93 100644 --- a/packages/api/src/agents/index.ts +++ b/packages/api/src/agents/index.ts @@ -1,6 +1,7 @@ export * from './avatars'; export * from './chain'; export * from './client'; +export * from './config'; export * from './context'; export * from './edges'; export * from './handlers';