📡 fix: Publish App-Level MCP Tool Catalogs Without a Reserved Revision - #14858
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0d99d9309
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d94695fd6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c48350eda1
ℹ️ 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".
Shared MCP servers advertised no tools to agents, so every turn failed with "configured to use MCP tools, but none are available" (#14857). `replaceAppServerTools` returned false whenever a publication carried no `publicationRevision`, but only `refreshChangedTools` reserves one. Every other app-level publisher — the first-connect snapshot, reinitialization, on-demand catalog reads, the retained-catalog restore — was silently dropped. The agent path fails closed on that drop: the skipped write returns null, so reinitialize yields no tools and the turn 503s. Startup hid it. `connectAppServers()` defers the initial refresh and calls `refreshToolList()` itself, which does reserve, so a boot that reaches its MCP servers looks healthy. Only a lazily created app connection — the server not yet up when LibreChat boots, a dropped connection, a cold cache — takes the unreserved path. `ConnectionsRepository` now reserves before its own `tools/list`, matching the list_changed path; a failed reservation publishes unordered rather than failing the connection. Publishers with no pre-fetch reservation point have already fetched by the time they reach the cache, so they take the next revision at write time instead of being discarded. `mergeAppTools` still publishes at revision 0 and stays deferential to a live catalog.
Addresses review feedback on the previous commit: allocating a revision at publish time lets a slow `tools/list` of an old catalog outrank a newer one that reserved after it started, and it would let the retained-catalog restore — which republishes deliberately pre-mutation data — outrank a live catalog. Ordering now travels with the data. `fetchToolsSnapshot` reserves before its first page and returns the ticket on the snapshot, so every app-level publisher reads the revision belonging to the read it is publishing rather than one allocated at an unrelated moment. `fetchOrderedToolsSnapshot` carries the refresh's revision when it defers to one, since that is whose catalog it returns. With the reservation at the single point where app-level tools are read, no publisher can forget it, so `replaceAppServerTools` goes back to refusing an unordered write: a publication that lost its ticket fetched at an unknown time and cannot be ordered. A failed reservation is reported as `orderingUnavailable` rather than swallowed, which keeps the list_changed path retrying instead of publishing a catalog that would be silently dropped, and leaves inspection unaffected by a transient cache outage. `MCPServerInspector.getToolFunctions` becomes `getToolCatalog` and returns the revision with the tools, so there is no variant that quietly discards ordering.
Review follow-up. The no-tools-capability branch destructured the reservation result and dropped `orderingUnavailable`, publishing without a revision when the revision store was transiently unavailable. That write is rejected in silence, and unlike the snapshot branch this one returned without reaching `refreshToolList()`, so whatever the server last advertised stayed in place until the connection was recreated or the cache expired. Both branches now route an unreservable catalog through the same retry path.
Review follow-up. Only the shared catalog write needs ordering; the tools themselves were just read from the server and are correct to serve. Discarding them because the write could not be ordered is what turns a cache failure into a server that appears to have no tools at all, which is the reported symptom. `updateMCPServerTools` now returns the tools it built when the publication has no reserved revision, instead of null. A superseded write still discards — there another replica holds something newer. Reinitialization also asks the connection to republish under backoff when its snapshot could not reserve ordering, so the shared catalog does not stay cold until something else triggers a refresh.
#14857 went a release without a diagnostic because the only trace of a dropped app-level catalog was a debug line no deployment runs. Operators saw agents fail every turn with nothing in the logs to explain it, and the reporter had to read the source to find the cause. A publication discarded because it cannot be addressed or ordered means this server's tools are unavailable to every agent that selected them, and serving an unpublished catalog means every request re-fetches it. Both are warnings now. A superseded write stays at debug: concurrent replicas produce it routinely and the winner already holds newer tools. Tests pin the level, so a later refactor cannot quietly make the failure silent again.
Reinitialization is the path an agent falls back to when the shared catalog is cold, so it is where #14857 surfaced as "configured to use MCP tools, but none are available". Nothing pinned that it forwards the ordering its snapshot was fetched with, nor that it asks the connection to republish a catalog it could not order. Both assertions fail against the pre-fix source.
3f91345 to
e495fcf
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ 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". |
Fixes #14857
Summary
Shared (app-level) MCP servers advertised no tools to agents on v0.8.8-rc1, so every turn failed with "The agent is configured to use MCP tools, but none are available." The reporter's root-cause analysis was correct.
replaceAppServerToolsreturnedfalsewhenever a publication carried nopublicationRevision, logging[MCP Cache] Skipped unordered app-level publication. OnlyMCPConnection.refreshChangedToolsever callsreserveMCPToolsChangedRevision, so every other app-level publisher was silently dropped:tools/listsnapshot inConnectionsRepository.loadConnectionreinitMCPServergetMCPTools(the/api/mcp/toolsendpoint)The agent path fails closed on that drop, which closes the loop to the user-visible error: cache miss →
reinitMCPServer→updateMCPServerTools→ skipped write → returnsnull→availableTools == null→ no tools → 503.Why it wasn't caught
Startup hides it.
connectAppServers()passesrefreshTools: falseand then callsrefreshToolList()itself, which does reserve a revision — so a boot that can reach its MCP servers looks completely healthy.Only a lazily created app connection takes the unreserved path: the MCP server isn't up yet when LibreChat boots (the normal Kubernetes/helm case, which is what the reporter has), the connection dropped, or the catalog aged out. That's why this reads as environment-specific while being 100% reproducible for the reporter.
The gate was introduced in #14686 (
1bccc2bc18), which shipped inv0.8.8-rc1and is not inv0.8.7— consistent with the reporter's rollback fixing it.Fix
Two layers, because the publishers differ in whether they can reserve ordering:
ConnectionsRepositoryreserves before its owntools/list— real pre-fetch ordering, mirroring the list_changed path. A failed reservation logs and publishes unordered rather than failing the connection.replaceAppServerToolsallocates a revision at write time when the caller had no pre-fetch reservation point. Publishers like reinitialize have already fetched by the time they reach the cache, so a publish-time revision is strictly better than discarding the write — it still can't clobber a newer committed catalog, and a stale in-flight publication still can't clobber it. The allocator (getNextAppToolsPublicationRevision) already existed and was exported; it just was never passed into the cache service.mergeAppToolsdeliberately still publishes at revision0so the startup inspection catalog stays deferential to a live one.Tests
7 new/changed tests across 3 files, including a real-store integration spec (
appToolPublication.integration.test.ts) that wires the actualcreateMCPCatalogStoreand reproduces the reporter's end-to-end symptom — a shared server republished by a user request must return its catalog rather thannull.All 7 were verified to fail against the un-fixed source and pass with the fix.
Full
packages/apiMCP suite: 61 suites / 1564 tests pass. Four suites fail identically on a clean tree and are unrelated —MCPReinitRecovery.integration.test.ts(getTenantId is not a function, staledata-schemasbuild) and three Redis-dependent*.cache_integration.spec.ts.