Conversation
![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: 3a5e830aae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| loader: () => Promise<ServerDefinition[]>, | ||
| configPaths: string[] | ||
| ): Promise<ServerDefinition[]> { | ||
| const cacheKey = configPaths.sort().join('|'); |
There was a problem hiding this comment.
Using configPaths.sort() mutates the caller-owned array before loader() executes. If the caller relies on path order for precedence (for example, later config layers overriding earlier ones) or reuses the same array elsewhere, this helper can silently change behavior while trying to cache. Build the key from a copied array so cache key generation is side-effect free.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| cache.set(cacheKey, { definitions, mtimes, timestamp: now }); |
There was a problem hiding this comment.
The cached timestamp is taken from now captured before await loader(), so slow loads consume part (or all) of the 5s TTL before the entry is even stored. When loading takes longer than the TTL, the next call immediately misses and reloads, defeating this optimization on exactly the slow environments it targets. Capture Date.now() at cache.set time instead.
Useful? React with 👍 / 👎.
1 participant
No description provided.