🔁 fix: Pass recursionLimit to OpenAI-Compatible Agents API Endpoint#12510
Conversation
… 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.
There was a problem hiding this comment.
Pull request overview
Fixes the OpenAI-compatible Agents API controller so LangGraph runs respect configured recursionLimit (matching the UI’s behavior), and updates client startup-config caching to separate unauthenticated vs authenticated startup-config entries.
Changes:
- Resolve and pass
recursionLimit(yaml default → per-agent DB override → max cap) intorun.processStream()for the OpenAI-compatible Agents endpoint. - Introduce an auth-aware
startupConfigKey(isAuthenticated)query key and update several cache reads to use the new key shape. - Adjust query enable/disable behavior around login to avoid stale config/query state across auth transitions.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/hooks/SSE/useEventHandlers.ts | Uses new startupConfigKey(...) when reading startup config from React Query cache. |
| client/src/hooks/Input/useQueryParams.ts | Uses new startupConfigKey(...) for model spec lookups from cached startup config. |
| client/src/hooks/Input/useQueryParams.spec.ts | Updates module mocking to preserve real exports (e.g., startupConfigKey). |
| client/src/hooks/Endpoint/useEndpoints.ts | Applies interface defaults via getConfigDefaults() when startup config lacks interface. |
| client/src/hooks/Conversations/useNavigateToConvo.tsx | Uses new startupConfigKey(...) when applying model spec effects. |
| client/src/hooks/Chat/useChatFunctions.ts | Uses new startupConfigKey(...) when reading startup config from cache. |
| client/src/hooks/AuthContext.tsx | Re-enables queries after successful auth context update. |
| client/src/data-provider/Endpoints/queries.ts | Adds startupConfigKey(isAuthenticated) and updates useGetStartupConfig query key. |
| client/src/data-provider/Auth/mutations.ts | Disables queries during login mutation; re-enables on error (success handled by AuthContext). |
| client/src/components/Nav/SettingsTabs/Data/ImportConversations.tsx | Uses new startupConfigKey(...) for startup config cache read (import size limit). |
| client/src/components/Chat/Menus/Endpoints/ModelSelector.tsx | Reuses a module-level interface default to avoid repeated getConfigDefaults() calls. |
| api/server/controllers/agents/openai.js | Resolves and passes recursionLimit to LangGraph processStream() config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Auth-aware query key so unauthenticated (login page) and authenticated | ||
| * (chat page) configs are cached independently, preventing stale | ||
| * unauthenticated config from persisting after login. | ||
| */ | ||
| export const startupConfigKey = (isAuthenticated: boolean) => | ||
| [QueryKeys.startupConfig, isAuthenticated] as const; | ||
|
|
||
| export const useGetStartupConfig = ( | ||
| config?: UseQueryOptions<t.TStartupConfig>, | ||
| ): QueryObserverResult<t.TStartupConfig> => { | ||
| const queriesEnabled = useRecoilValue<boolean>(store.queriesEnabled); | ||
| const user = useRecoilValue<t.TUser | undefined>(store.user); | ||
| return useQuery<t.TStartupConfig>( | ||
| [QueryKeys.startupConfig], | ||
| startupConfigKey(!!user), | ||
| () => dataService.getStartupConfig(), |
There was a problem hiding this comment.
startupConfigKey is documented/typed as taking an isAuthenticated boolean, but useGetStartupConfig currently derives it from !!user. Several call sites in this PR pass startupConfigKey(true) when reading from the cache, which will not match the query key while user is still undefined and can lead to missing/incorrect startupConfig reads. Consider standardizing the key derivation (e.g., base it on an explicit auth flag) or provide a helper that retrieves startupConfig regardless of the auth-suffixed key so callers can’t accidentally diverge.
| sourceId: submission.conversation?.conversationId, | ||
| ephemeralAgent: submission.ephemeralAgent, | ||
| specName: submission.conversation?.spec, | ||
| startupConfig: queryClient.getQueryData<TStartupConfig>([QueryKeys.startupConfig]), | ||
| startupConfig: queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)), | ||
| }); |
There was a problem hiding this comment.
This cache lookup hard-codes startupConfigKey(true), but the startup config query key is generated as startupConfigKey(!!user) in useGetStartupConfig. If user is not yet set (or is temporarily undefined), this will miss the cached startup config. Prefer deriving the boolean from the same source (e.g., !!queryClient.getQueryData([QueryKeys.user])) or using a shared helper to read startupConfig from the cache consistently.
| } | ||
| let newPreset = removeUnavailableTools(_newPreset, availableTools); | ||
| if (newPreset.spec != null && newPreset.spec !== '') { | ||
| const startupConfig = queryClient.getQueryData<TStartupConfig>([QueryKeys.startupConfig]); | ||
| const startupConfig = queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)); | ||
| const modelSpecs = startupConfig?.modelSpecs?.list ?? []; | ||
| const spec = modelSpecs.find((s) => s.name === newPreset.spec); |
There was a problem hiding this comment.
This cache lookup hard-codes startupConfigKey(true), but useGetStartupConfig keys the query as startupConfigKey(!!user). If user is undefined at the time this runs, getQueryData will miss and model spec validation may behave incorrectly. Suggest deriving the boolean from the cached user (or using a shared helper) so cache reads always match the query key.
| return; | ||
| } | ||
|
|
||
| const startupConfig = queryClient.getQueryData<TStartupConfig>([QueryKeys.startupConfig]); | ||
| const startupConfig = queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)); | ||
| applyModelSpecEffects({ |
There was a problem hiding this comment.
This cache lookup uses startupConfigKey(true); however, useGetStartupConfig keys startupConfig by startupConfigKey(!!user). To avoid missing cache reads during auth transitions, derive the boolean from the same source (e.g., the cached user) or centralize startupConfig cache access behind a helper.
| @@ -172,7 +173,7 @@ export default function useChatFunctions({ | |||
| } | |||
|
|
|||
| const endpointsConfig = queryClient.getQueryData<TEndpointsConfig>([QueryKeys.endpoints]); | |||
| const startupConfig = queryClient.getQueryData<TStartupConfig>([QueryKeys.startupConfig]); | |||
| const startupConfig = queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)); | |||
There was a problem hiding this comment.
This cache lookup hard-codes startupConfigKey(true), but useGetStartupConfig uses startupConfigKey(!!user) for the query key. If user isn’t set yet, this will return undefined and downstream endpoint param logic may run without startupConfig. Consider deriving the key boolean from cached user/isAuthenticated, or use a shared cache accessor that tries both keys.
| const startupConfig = queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)); | |
| const startupConfig = | |
| queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)) ?? | |
| queryClient.getQueryData<TStartupConfig>(startupConfigKey(false)); |
| const handleFileUpload = useCallback( | ||
| async (file: File) => { | ||
| try { | ||
| const startupConfig = queryClient.getQueryData<TStartupConfig>([QueryKeys.startupConfig]); | ||
| const startupConfig = queryClient.getQueryData<TStartupConfig>(startupConfigKey(true)); | ||
| const maxFileSize = startupConfig?.conversationImportMaxFileSize; |
There was a problem hiding this comment.
This cache lookup hard-codes startupConfigKey(true), but the startupConfig query is keyed by startupConfigKey(!!user) in useGetStartupConfig. If user is undefined (e.g., during auth refresh transitions), the startupConfig lookup may fail and file-size validation will be skipped. Prefer deriving the boolean from the cached user or using a shared helper for startupConfig cache reads.
e4c5a9f to
301bece
Compare
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.
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.
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.
Use mockReturnValueOnce instead of mockReturnValue to prevent mock leaking across test boundaries. Add boundary tests for downward agent override and exact-match maxRecursionLimit.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…anny-avila#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.
…anny-avila#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.
Summary
Resolves #12476
The OpenAI-compatible Agents API endpoint (
openai.js) never passedrecursionLimitto LangGraph'sprocessStream(), so all API-based agent calls were silently capped at LangGraph's default of 25 steps (~8 tool calls). The UI path (client.js) had the correct logic but it was duplicated inline.resolveRecursionLimit()utility in@librechat/api(packages/api/src/agents/config.ts) that implements the 3-step cascade: yaml config default (fallback 50) → per-agent DB override → global max caprecursion_limit=0ormaxRecursionLimit=0would silently misbehave, by using explicittypeof+ positive-number checks instead of truthinessopenai.jsto call the shared utility, hoistedagentsEConfigto avoid a duplicate property walk forallowedProvidersclient.jsto replace the inline cascade with the shared utility callopenai.spec.jsverifyingprocessStreamreceives the resolvedrecursionLimitChange Type
Testing
Since this affects the OpenAI-compatible API endpoint for agents, testing should be done via programmatic API calls:
/v1/chat/completionswith an agent model)recursion_limitset on the agent in the database and verify it is respectedmaxRecursionLimitset in yaml config and verify the cap is enforcedChecklist