mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-04-03 06:17:21 +02:00
🔁 fix: Pass recursionLimit to OpenAI-Compatible Agents API Endpoint (#12510)
* 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.
This commit is contained in:
parent
aa575b274b
commit
cb41ba14b2
6 changed files with 134 additions and 18 deletions
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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) =>
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
62
packages/api/src/agents/config.spec.ts
Normal file
62
packages/api/src/agents/config.spec.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
30
packages/api/src/agents/config.ts
Normal file
30
packages/api/src/agents/config.ts
Normal file
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue