Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 1f840d51e0
ℹ️ 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".
| const agent = applyLoaded(agentId, outcome.loaded); | ||
| if (agent?.edges?.length) { | ||
| collectEdges(agent.edges); |
There was a problem hiding this comment.
When the primary agent references both B and C, and B also references C, applying B's result calls collectEdges before C has been added to agentConfigs. Because the level's queue was cleared, the collector queues C again, and the next iteration initializes it a second time. I reproduced initialization order B, C, C using the changed discovery function. This repeats database, permission, tool-loading, and initialization callback work and overwrites C's first configuration. Track agents already staged in the current level, or apply all loaded configurations before collecting their edges.
Useful? React with 👍 / 👎.
| /** Bound on concurrent per-server catalog lookups when assembling an ephemeral agent's tools. */ | ||
| export const MCP_CATALOG_LOOKUP_CONCURRENCY = 4; |
There was a problem hiding this comment.
The new MCP catalog concurrency limit is hard-coded, as are the new skill-name and resume-agent lookup limits, with no corresponding settings in configSchema. Operators therefore cannot reduce the newly parallel workload for constrained database or MCP deployments without modifying code. AGENTS.md explicitly requires newly introduced limits to be configurable through librechat.yaml; add schema-backed settings with defaults and pass the resolved values into these loaders.
AGENTS.md reference: AGENTS.md:L91-L93
Useful? React with 👍 / 👎.
| * the node limit, de-duplication and first-error semantics match a serial walk. */ | ||
| const unseenIds = [...new Set(referencedIds)].filter((id) => !byId.has(id)); | ||
| const fetched = new Map<string, { agent: Agent | null } | { error: unknown }>(); | ||
| await mapWithConcurrency(unseenIds, RESUME_AGENT_LOAD_CONCURRENCY, async (id) => { |
There was a problem hiding this comment.
When a saved agent references more accessible agents than the 50-node traversal limit, this batch fetches and permission-checks every reference before the result-processing loop can enforce that limit. Agent validation permits an unbounded edges array, so a large saved graph now causes database work proportional to the entire array even though the resume will be rejected. Running the snapshot function with 1,000 accessible references produced 1,000 agent reads after this change versus 50 before it. Schedule bounded batches that stop fetching once the traversal budget is exceeded rather than checking the budget only after all lookups finish.
Useful? React with 👍 / 👎.
|
|
||
| const detail = params.imageDetail ?? req.body.imageDetail ?? ImageDetail.auto; | ||
| const formattedImages = await Promise.all(promises); | ||
| const settled = await Promise.allSettled(promises); |
There was a problem hiding this comment.
If an early blob attachment fails to load while a later attachment's download stalls, Promise.allSettled prevents the known error from reaching the caller until the later download finishes. Previously, the early blob failure rejected immediately. This can leave the chat request waiting on an irrelevant storage read: getFileStream buffers the stream without passing a cancellation signal, and the cross-region CloudFront stream clears its fetch timeout after receiving headers. Await the already-started, rejection-tracked promises in attachment order so the first ordered failure can propagate without waiting for subsequent reads.
Useful? React with 👍 / 👎.
Summary
Removes serial awaits on chat and schedule request paths found by the 2026-10-19 Devin performance scan. Connected agents in
discoverConnectedAgentswere initialized one at a time; MCP tool catalogs in the ephemeral and added-agent loaders, model-spec skill name lookups (up to 100), skill file inspection, resume snapshot agent lookups, blob and remote image reads during encoding, schedule enrollment resolution and the authorize loops in schedule preflight and bearer minting all awaited independent work in sequence.Each now runs with bounded concurrency through a new
mapWithConcurrencyhelper built on the existingcreateConcurrencyLimiter. Results are applied in the original order, so tool order, theuserMCPAuthMapmerge, agent config order, dedupe, depth/node limits and first-error selection match the serial version. The prompt migration check also starts its independent role, project and ACL reads together.Type of change
Testing
Tested environments/configuration: Node 20, packages/api unit specs.
Automated tests:
mapWithConcurrencyand overlapping image reads.packages/apijest across discovery, skills, hitl, prompts, schedules, agents and files/encode: 58 suites, 2079 passed, 4 skipped.tsc --noEmitinpackages/apiclean.Screenshots / recordings
No user-facing change.
Risk / compatibility
Within one BFS level,
onAgentInitializedcallbacks now fire in completion order; the state they feed is still applied in queue order. In bearer minting, the surfaced denial is the first to reject rather than the first in selection order, as the validate pass already behaved. Skill archive import, retained-answer reads and admin config pagination were left serial or unbounded on purpose (storage quota reservation, full-branch reads, wire shape).Checklist