🧊 fix: In-Memory Endpoint Token Config Cache Isolation - #12673
Merged
Merged
Conversation
…itializing clients)
- Introduced a constant `DEFAULT_MAX_CONTEXT_TOKENS` set to 32000. - Updated the `initializeAgent` function to use this constant instead of hardcoded values for maximum context tokens, improving maintainability and clarity.
- Introduced a memoized in-memory cache for Keyv instances to ensure shared access across the same namespace, improving cache efficiency. - Updated the `standardCache` function to utilize the new in-memory cache for the TOKEN_CONFIG namespace. - Refactored the `initializeCustom` function to use the `tokenConfigCache` for better cache management. - Removed redundant tokenCache parameter from `fetchModels` to streamline the function signature.
Collaborator
Author
|
@codex review |
danny-avila
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffd3440697
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Comment on lines
+91
to
+93
| const existing = inMemoryCacheMap.get(namespace); | ||
| if (existing) { | ||
| return existing; |
There was a problem hiding this comment.
Returning an existing in-memory Keyv instance by namespace without considering the requested ttl makes cache expiration order-dependent. TOKEN_CONFIG is now accessed both without TTL (tokenConfigCache()) and with a 30-minute TTL (api/cache/getLogStores.js); if the no-TTL path runs first (e.g., package-level initialization/tests before legacy cache bootstrapping), subsequent TTL-enabled calls silently reuse the no-expiry instance, so token config entries never expire and can remain stale indefinitely.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes endpoint token configuration caching so that token-config writes and reads share the same cache instance when Redis is disabled/forced in-memory, preventing “empty cache” behavior across call sites.
Changes:
- Added in-memory
Keyvinstance memoization tostandardCache()and introduced atokenConfigCache()convenience accessor forCacheKeys.TOKEN_CONFIG. - Updated token-config caching call sites to use
tokenConfigCache()(custom endpoint initialization and model fetch flow). - Changed agent default max-context fallback from
18000to32000tokens via a shared constant.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/api/src/endpoints/models.ts | Writes endpoint token-config into the dedicated token-config cache accessor. |
| packages/api/src/endpoints/custom/initialize.ts | Reads token-config from the dedicated token-config cache accessor to avoid cache-instance mismatch. |
| packages/api/src/cache/cacheFactory.ts | Memoizes in-memory caches by namespace and adds tokenConfigCache() helper. |
| packages/api/src/agents/initialize.ts | Replaces hardcoded max-context fallback with a constant and changes its value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const endpointTokenConfig = processModelData(input); | ||
| const cache = standardCache(CacheKeys.TOKEN_CONFIG); | ||
| await cache.set(tokenKey ?? name, endpointTokenConfig); | ||
| await tokenConfigCache().set(tokenKey ?? name, endpointTokenConfig); |
| maxToolResultChars?: number; | ||
| }; | ||
|
|
||
| const DEFAULT_MAX_CONTEXT_TOKENS = 32000; |
Comment on lines
+71
to
+72
| const DEFAULT_MAX_CONTEXT_TOKENS = 32000; | ||
|
|
Comment on lines
+22
to
+26
| /** | ||
| * Memoized in-memory Keyv instances keyed by namespace. | ||
| * Without Redis, each `new Keyv()` gets its own internal Map, so callers that | ||
| * write in one call-site and read in another would see an empty store. | ||
| * Memoizing ensures a single shared Map per namespace across the entire bundle. |
Pass Time.THIRTY_MINUTES to tokenConfigCache() to match the TTL used by getLogStores.js, preventing load-order-dependent expiry behavior. Add 7 automated tests covering in-memory memoization: referential identity, cross-call-site data sharing, namespace isolation, first-caller TTL semantics, fallbackStore bypass, and tokenConfigCache parity with direct standardCache access.
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ 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". |
Contributor
GitNexus: ❌ deploy failedThe deploy failed — the previous index (if any) continues to be served. |
- Export the constant so tests (and future consumers) reference it directly instead of hardcoding the numeric value. - Add independent TTL assertion for tokenConfigCache (R-1 nit). - Add tokenConfigCache mock to custom/initialize.spec.ts.
Contributor
GitNexus: ❌ deploy failedThe deploy failed — the previous index (if any) continues to be served. |
danny-avila
changed the title
krgokul
pushed a commit
to syedhabib39/LibreChat
that referenced
this pull request
Apr 20, 2026
…12673) * fix: endpoint token config not using shared cache in same process (initializing clients) * refactor: Update default max context tokens for agent initialization - Introduced a constant `DEFAULT_MAX_CONTEXT_TOKENS` set to 32000. - Updated the `initializeAgent` function to use this constant instead of hardcoded values for maximum context tokens, improving maintainability and clarity. * refactor: shared caching mechanism for token configuration - Introduced a memoized in-memory cache for Keyv instances to ensure shared access across the same namespace, improving cache efficiency. - Updated the `standardCache` function to utilize the new in-memory cache for the TOKEN_CONFIG namespace. - Refactored the `initializeCustom` function to use the `tokenConfigCache` for better cache management. - Removed redundant tokenCache parameter from `fetchModels` to streamline the function signature. * fix: match TOKEN_CONFIG TTL and add memoization tests Pass Time.THIRTY_MINUTES to tokenConfigCache() to match the TTL used by getLogStores.js, preventing load-order-dependent expiry behavior. Add 7 automated tests covering in-memory memoization: referential identity, cross-call-site data sharing, namespace isolation, first-caller TTL semantics, fallbackStore bypass, and tokenConfigCache parity with direct standardCache access. * fix: export DEFAULT_MAX_CONTEXT_TOKENS, address review nits - Export the constant so tests (and future consumers) reference it directly instead of hardcoding the numeric value. - Add independent TTL assertion for tokenConfigCache (R-1 nit). - Add tokenConfigCache mock to custom/initialize.spec.ts.
krgokul
pushed a commit
to syedhabib39/LibreChat
that referenced
this pull request
Apr 21, 2026
…12673) * fix: endpoint token config not using shared cache in same process (initializing clients) * refactor: Update default max context tokens for agent initialization - Introduced a constant `DEFAULT_MAX_CONTEXT_TOKENS` set to 32000. - Updated the `initializeAgent` function to use this constant instead of hardcoded values for maximum context tokens, improving maintainability and clarity. * refactor: shared caching mechanism for token configuration - Introduced a memoized in-memory cache for Keyv instances to ensure shared access across the same namespace, improving cache efficiency. - Updated the `standardCache` function to utilize the new in-memory cache for the TOKEN_CONFIG namespace. - Refactored the `initializeCustom` function to use the `tokenConfigCache` for better cache management. - Removed redundant tokenCache parameter from `fetchModels` to streamline the function signature. * fix: match TOKEN_CONFIG TTL and add memoization tests Pass Time.THIRTY_MINUTES to tokenConfigCache() to match the TTL used by getLogStores.js, preventing load-order-dependent expiry behavior. Add 7 automated tests covering in-memory memoization: referential identity, cross-call-site data sharing, namespace isolation, first-caller TTL semantics, fallbackStore bypass, and tokenConfigCache parity with direct standardCache access. * fix: export DEFAULT_MAX_CONTEXT_TOKENS, address review nits - Export the constant so tests (and future consumers) reference it directly instead of hardcoding the numeric value. - Add independent TTL assertion for tokenConfigCache (R-1 nit). - Add tokenConfigCache mock to custom/initialize.spec.ts.
yidianyiko
pushed a commit
to yidianyiko/LibreChat
that referenced
this pull request
May 6, 2026
…12673) * fix: endpoint token config not using shared cache in same process (initializing clients) * refactor: Update default max context tokens for agent initialization - Introduced a constant `DEFAULT_MAX_CONTEXT_TOKENS` set to 32000. - Updated the `initializeAgent` function to use this constant instead of hardcoded values for maximum context tokens, improving maintainability and clarity. * refactor: shared caching mechanism for token configuration - Introduced a memoized in-memory cache for Keyv instances to ensure shared access across the same namespace, improving cache efficiency. - Updated the `standardCache` function to utilize the new in-memory cache for the TOKEN_CONFIG namespace. - Refactored the `initializeCustom` function to use the `tokenConfigCache` for better cache management. - Removed redundant tokenCache parameter from `fetchModels` to streamline the function signature. * fix: match TOKEN_CONFIG TTL and add memoization tests Pass Time.THIRTY_MINUTES to tokenConfigCache() to match the TTL used by getLogStores.js, preventing load-order-dependent expiry behavior. Add 7 automated tests covering in-memory memoization: referential identity, cross-call-site data sharing, namespace isolation, first-caller TTL semantics, fallbackStore bypass, and tokenConfigCache parity with direct standardCache access. * fix: export DEFAULT_MAX_CONTEXT_TOKENS, address review nits - Export the constant so tests (and future consumers) reference it directly instead of hardcoding the numeric value. - Add independent TTL assertion for tokenConfigCache (R-1 nit). - Add tokenConfigCache mock to custom/initialize.spec.ts. (cherry picked from commit a613cac)
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
…12673) * fix: endpoint token config not using shared cache in same process (initializing clients) * refactor: Update default max context tokens for agent initialization - Introduced a constant `DEFAULT_MAX_CONTEXT_TOKENS` set to 32000. - Updated the `initializeAgent` function to use this constant instead of hardcoded values for maximum context tokens, improving maintainability and clarity. * refactor: shared caching mechanism for token configuration - Introduced a memoized in-memory cache for Keyv instances to ensure shared access across the same namespace, improving cache efficiency. - Updated the `standardCache` function to utilize the new in-memory cache for the TOKEN_CONFIG namespace. - Refactored the `initializeCustom` function to use the `tokenConfigCache` for better cache management. - Removed redundant tokenCache parameter from `fetchModels` to streamline the function signature. * fix: match TOKEN_CONFIG TTL and add memoization tests Pass Time.THIRTY_MINUTES to tokenConfigCache() to match the TTL used by getLogStores.js, preventing load-order-dependent expiry behavior. Add 7 automated tests covering in-memory memoization: referential identity, cross-call-site data sharing, namespace isolation, first-caller TTL semantics, fallbackStore bypass, and tokenConfigCache parity with direct standardCache access. * fix: export DEFAULT_MAX_CONTEXT_TOKENS, address review nits - Export the constant so tests (and future consumers) reference it directly instead of hardcoding the numeric value. - Add independent TTL assertion for tokenConfigCache (R-1 nit). - Add tokenConfigCache mock to custom/initialize.spec.ts.
ThomasVuNguyen
pushed a commit
to ThomasVuNguyen/LibreChat
that referenced
this pull request
Jul 15, 2026
…12673) * fix: endpoint token config not using shared cache in same process (initializing clients) * refactor: Update default max context tokens for agent initialization - Introduced a constant `DEFAULT_MAX_CONTEXT_TOKENS` set to 32000. - Updated the `initializeAgent` function to use this constant instead of hardcoded values for maximum context tokens, improving maintainability and clarity. * refactor: shared caching mechanism for token configuration - Introduced a memoized in-memory cache for Keyv instances to ensure shared access across the same namespace, improving cache efficiency. - Updated the `standardCache` function to utilize the new in-memory cache for the TOKEN_CONFIG namespace. - Refactored the `initializeCustom` function to use the `tokenConfigCache` for better cache management. - Removed redundant tokenCache parameter from `fetchModels` to streamline the function signature. * fix: match TOKEN_CONFIG TTL and add memoization tests Pass Time.THIRTY_MINUTES to tokenConfigCache() to match the TTL used by getLogStores.js, preventing load-order-dependent expiry behavior. Add 7 automated tests covering in-memory memoization: referential identity, cross-call-site data sharing, namespace isolation, first-caller TTL semantics, fallbackStore bypass, and tokenConfigCache parity with direct standardCache access. * fix: export DEFAULT_MAX_CONTEXT_TOKENS, address review nits - Export the constant so tests (and future consumers) reference it directly instead of hardcoding the numeric value. - Add independent TTL assertion for tokenConfigCache (R-1 nit). - Add tokenConfigCache mock to custom/initialize.spec.ts.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Without Redis,
standardCache()created a new in-memoryKeyvinstance on every call. Each instance has its own isolatedMap, so data written by one call-site (e.g.,fetchModelsat startup) was invisible to reads from another call-site (e.g.,initializeCustomduring agent init). This meantendpointTokenConfigfor providers like OpenRouter was never available when initializing agents, causing a redundantfetchModelsHTTP call on every request and the token config being absent for usage tracking.Changes
packages/api/src/cache/cacheFactory.tsstandardCachenow memoizes in-memoryKeyvinstances by namespace (viainMemoryCacheMap). Repeated calls with the same namespace return the same instance, so writes and reads share the same store. This only applies to the plain in-memory path (no Redis, no customfallbackStore). Redis-backed instances are unaffected since they already share a backend.tokenConfigCache()convenience accessor for theTOKEN_CONFIGnamespace.packages/api/src/endpoints/models.tsfetchModelsnow writes token config viatokenConfigCache()instead of creating an ad-hocstandardCacheinstance.packages/api/src/endpoints/custom/initialize.tsinitializeCustomnow reads token config viatokenConfigCache(), ensuring it sees data written at startup byfetchModels.packages/api/src/agents/initialize.ts18000fallback with a namedDEFAULT_MAX_CONTEXT_TOKENSconstant (32000).Test plan
standardCachenamespace isolation and cache integration tests pass (10/10)endpointTokenConfigon first request (no extra HTTP call to/models)