🛡️ fix: Handle MCP Tool Cache Lookup Failures - #12910
Conversation
There was a problem hiding this comment.
Pull request overview
Makes the MCP tools listing endpoint resilient to partial cache failures so one broken MCP server/tool-cache read doesn’t take down /api/mcp/tools for all servers, preserving existing live-manager fallback behavior and adding coverage to prevent regression.
Changes:
- Wrap per-server MCP tool cache reads in a try/catch so a single cache rejection doesn’t fail the entire request.
- Continue falling back to
mcpManager.getServerToolFunctionswhen cached tools can’t be read. - Add a route test asserting tools from a healthy server are still returned when another server’s cache lookup fails.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| api/server/controllers/mcp.js | Isolates per-server cache lookup failures and preserves per-server live fallback behavior. |
| api/server/routes/tests/mcp.spec.js | Adds regression test for /api/mcp/tools when one server’s cached tools lookup rejects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GitNexus: 🚀 deployedThe |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ 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". |
GitNexus: 🚀 deployedThe |
GitNexus: 🚀 deployedThe |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ 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". |
* Handle MCP tool cache lookup failures * Harden MCP cached tool lookup * Cover full MCP tool cache outage * Guard MCP tool cache store lookup
Adds an "Upstream context and related work" section pointing at relevant work on danny-avila/LibreChat: - PR LibreChat-AI#11799 + Issue LibreChat-AI#10641: direct overlap with this plan's MCP Apps work; lists the 10 axes on which v8 intentionally diverges (self-contained HTML, no direct browser networking, one proxy per instance, hop-specific relay validation, manifest-hash approval, etc.). - Issue LibreChat-AI#11997: the only upstream artifact for MCP Tasks (no PR yet); plan's Tasks work is greenfield. - PR LibreChat-AI#12850: 307/308 redirect handling and credential stripping is merged into dev but NOT in HEAD 738003b. Earlier revisions of this plan claimed main already had it; corrected. Phase 0 now tracks the upstream merge or ports the work if it slips. - PR LibreChat-AI#12535, LibreChat-AI#12853, LibreChat-AI#12910, Issue LibreChat-AI#12802: adjacent transport reliability and OAuth hygiene worth tracking. - PRs already in HEAD listed for context (LibreChat-AI#12782, LibreChat-AI#12763, LibreChat-AI#12755, LibreChat-AI#12745, LibreChat-AI#12812). - Notably absent upstream: session-id reuse correctness, header consistency, 404 → re-init, per-user token scoping, outstanding-task revalidation, legacy renderer retirement. References section reorganized into Specs / Upstream LibreChat / MDN subsections. https://claude.ai/code/session_011NZqb4xN9QcXpdY2LCtnuH
…s Hardened split) Restructures the implementation strategy after the eighth review: - Stop parallel-building. Phase 1 inherits upstream PR LibreChat-AI#11799 as the Apps Preview substrate (per-instance outer iframes, same-server binding, /api/mcp/sandbox auth, ui:// URI validation, capability gating, mcp_app artifact, test server, stableMCPAppRef) and verifies these with regression tests rather than re-implementing them. - Split Apps phasing into: * Phase 2P (Apps Preview, ~1-2w): adopt LibreChat-AI#11799 substantially as-is on chat surface only; consolidate to single ACL; truthful HostContext; payload truncation; rate limits; fullscreen behind appSettings.allowFullscreen; capability advertise behind MCP_APPS_PREVIEW_ENABLED. * Phase 2H (Apps Hardened GA, +2-3w later release): layer the v8 security delta - dedicated MCP_SANDBOX_ORIGIN, explicit-target-origin transport, hop-specific relay + proxy-stamped nonce, folded-in ui/initialize probe, connect-src 'none', self-contained HTML, MCPAppLaunchManifest review, MCPAppInstance write path, full UIResourceRenderer retirement across all surfaces - behind MCP_APPS_HARDENED_ENABLED. - Narrow first Apps release to live chat only. Share, search, and plugin-rendered surfaces fall back to text in both Preview and Hardened GA. Cross-surface support is post-Hardened-GA. - Defer MCPAppInstance full-conversation remount to Hardened GA. Preview relies on upstream stableMCPAppRef for parent- re-render survival. - Accept upstream fullscreen support behind appSettings.allowFullscreen (default OFF per server) instead of stripping in delta. - Trim Tasks v1 to status-first jobs panel. Progress bars deferred until upstream PR LibreChat-AI#12535 lands; reuse rather than rebuild. Tasks v1 is independent of Apps tracks. - Phase 0 starts from dev (inheriting PRs LibreChat-AI#12850, LibreChat-AI#12853, LibreChat-AI#12910) rather than older main HEAD; remaining work is session reuse, header consistency, basic 404 -> re-init, per-user token scoping, authContextHash. - Three independent feature flags: MCP_APPS_PREVIEW_ENABLED, MCP_APPS_HARDENED_ENABLED, MCP_TASKS_ENABLED. Old MCP_APPS_ENABLED becomes a deprecated alias for Preview with a startup warning. - Default-deny sandboxPermissions on the legacy renderer applies even when both Apps flags are off. Updated changelog header, Closed go/no-go decisions (added 19/20/21), Carried-forward list with track tags, current-state table with track markers, phases (Phase 0/1/2P/2H/4), test matrix (P/H/A tags), risks, and effort summary (~3-4w for Preview+Tasks; full v1 ~5-7w). References section unchanged. https://claude.ai/code/session_011NZqb4xN9QcXpdY2LCtnuH
* Handle MCP tool cache lookup failures * Harden MCP cached tool lookup * Cover full MCP tool cache outage * Guard MCP tool cache store lookup
* Handle MCP tool cache lookup failures * Harden MCP cached tool lookup * Cover full MCP tool cache outage * Guard MCP tool cache store lookup
Summary
GET /api/mcp/toolstolerate per-server cache lookup failures.getMCPServerToolsitself so all callers degrade to cache-miss behavior when the tool cache backend is unavailable.getMCPServerTools.Root Cause
The MCP tools endpoint gathered cached tools for every configured server with a single aggregate promise. If one server's cache read rejected, the whole endpoint returned a 500 before later per-server error isolation could run. The same helper was also used by agent-loading paths, so centralizing the fallback in
getMCPServerToolsprevents cache read failures from breaking those callers as well.Review Resolution
getMCPServerToolscentrally.nulland log the error.getLogStoresinside the helper guard and adding coverage for cache-store resolution failure.Validation
node --check api/server/controllers/mcp.jsnode --check api/server/routes/__tests__/mcp.spec.jsnode --check api/server/services/Config/getCachedTools.jsnode --check api/server/services/Config/__tests__/getCachedTools.spec.jsgit diff --check -- api/server/controllers/mcp.js api/server/routes/__tests__/mcp.spec.js api/server/services/Config/getCachedTools.js api/server/services/Config/__tests__/getCachedTools.spec.jsAttempted:
cd api && npx jest server/routes/__tests__/mcp.spec.js --runInBandcd api && npx jest server/services/Config/__tests__/getCachedTools.spec.js --runInBandBoth Jest runs failed before executing tests because this worktree has no installed dependencies and Jest setup cannot resolve
dotenvfromapi/test/jestSetup.js.