Files
sim/apps
WaleedandClaude Opus 4.7 0c96964f8f improvement(mcp): per-server tool queries + negative cache (#4715)
* improvement(mcp): per-server tool queries + negative cache so one slow server can't block the workspace

Move MCP tool discovery off the workspace-aggregated `Promise.all` fan-out and
onto per-server React Query keys, matching how Cursor and Claude Code render
remote MCP. `useMcpToolsQuery` is now a `useQueries` combiner: each server has
its own cache entry, its own loading state, and a slow neighbor never gates the
others. Public shape stays compatible with existing consumers.

Add a short-TTL negative cache: when `listTools` fails (timeout, connection
error, etc.) we mark the server unhealthy for 30s so subsequent discovery calls
short-circuit instead of re-paying the timeout. OAuth-required errors are
exempt so re-auth retries immediately. Drop `LIST_TOOLS_TIMEOUT_MS` from 30s
to 10s to bound the worst-case first failure.

Invalidations are per-server where the action is per-server (OAuth popup,
per-server SSE event, refresh, update, delete). Bulk operations stay
workspace-broad. Adds tests for the negative-cache behavior and the OAuth
exemption.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* improvement(mcp): in-flight dedup + 2min negative TTL + no refetch on window focus

Three small follow-ups on top of per-server tool queries:

- Coalesce concurrent `discoverServerTools(userId, serverId, workspaceId)`
  calls into a single upstream `tools/list`. Races between OAuth-callback
  cache priming, post-OAuth UI refetch, and multi-tab loads no longer
  double-fetch the same server.
- Bump negative-cache TTL from 30s to 2 minutes. Cleared on listChanged,
  OAuth completion, manual refresh, and the next successful discovery, so
  this floor only matters for genuinely dead servers — drops their floor
  traffic by 4x.
- Disable `refetchOnWindowFocus` on per-server tool queries. listChanged
  SSE + mutation invalidations already cover real schema changes; alt-tab
  no longer triggers N parallel `tools/list` calls.

* fix(mcp): address bugbot/greptile review on per-server tool discovery

- Workspace-scoped `mcpKeys.serverToolsWorkspace(workspaceId)` prefix for bulk
  invalidations (create-server, refresh-all, SSE workspace fallback, OAuth
  fallback). The previous `mcpKeys.serverTools()` prefix was global and
  invalidated every workspace's tools cache.
- `useMcpToolsQuery` folds `useMcpServers().isLoading` into the aggregate
  `isLoading` so mounting no longer flashes an "empty tools" state during the
  servers-list fetch. Aggregate `error` is suppressed when any per-server
  query already returned data so one slow server can't blank out the others.
- `useForceRefreshMcpTools` invalidates the per-server query keys of servers
  whose force refresh failed, so stale tools don't linger.
- `DiscoveryOutcome` error variant carries the original error, restoring the
  OAuth-exemption check that `getErrorMessage(...)` previously erased.
- `discoverServerTools(userId, serverId, workspaceId, forceRefresh = false)`
  now consults the positive + negative cache by default. Per-server React
  Query refetches hit the cache instead of re-paying the listTools timeout;
  callers that explicitly bypass cache (refresh route, OAuth callback, bulk
  POST refresh) pass `forceRefresh: true`. Negative-cache hits throw a typed
  `McpConnectionError` so the route layer can surface a fast 503.

* update icons

* chore(mcp): remove dead query keys and trim verbose comments

- Drop unused `mcpKeys.tools()` / `mcpKeys.toolsList()` — replaced by
  per-server keys, no remaining callers.
- Trim narrative comments to keep only the non-obvious "why" notes.

* fix(mcp): map negative-cache cooldown error to HTTP 503

`McpConnectionError` thrown when a server is in cooldown previously
fell through `categorizeError` to a generic 500. Cooldown is a
transient-unavailability condition, so route it to 503.

* test(mcp): cover cooldown error → 503 categorization

* fix(mcp): address second-round bugbot review

- useMcpToolsQuery serverIds: filter on enabled + workspaceId match.
  Disabled rows no longer trigger discover calls that get negative-cached,
  and keepPreviousData on useMcpServers no longer races a workspace switch
  into cross-workspace discover requests.
- Aggregate skips per-server data when that server's latest refetch errored,
  so a broken server's last-known tools no longer linger in the workspace
  view while its card shows an error.
- discoverServerTools failure path drops the positive cache alongside writing
  the negative-cache marker. A cache-respecting follow-up now fails fast via
  cooldown instead of returning stale tools from a now-broken server.
- useMcpTools.refreshTools drops the dead forceRefresh param — the per-server
  queryFn always sends refresh=false, so the flag was never effective. Callers
  wanting cache-bypass should use useForceRefreshMcpTools.

* chore(mcp): trim verbose comments

* fix(mcp): third-round bugbot review

- discoverTools failure path now drops the per-server positive cache alongside
  writing the negative-cache marker, matching discoverServerTools' behavior so
  a workspace-aggregate failure doesn't leave stale tools cached.
- useForceRefreshMcpTools filters disabled and out-of-workspace rows before
  fan-out so disabled servers don't 404 → negative-cache themselves.
- Remove unused useMcpServerTools export — the aggregate goes through
  useQueries directly, no external consumer exists.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-21 22:40:11 -07:00
..