iframe-proxy

waleedlatif1 · 2026-10-21T04:07:51Z

Summary

Broad chat catalog discovery (lib/mothership/integrations/application/catalog.ts) passes every usable workspace server to buildMcpToolSchemas as best-effort, and lib/mothership/mcp-tools.ts started all of them at once with Promise.allSettled. withinDeadline only ends the caller's 3s wait. The attempt itself keeps running (connect up to 60s, tools/list up to 60s, plus a retry), so a large workspace opens an unbounded burst of sockets. The connection pool caps retained connections, not pending ones, so a burst of more than 100 LRU-evicts other users' warm connections. Best-effort failures write no shared cooldown, which means a down server was reattempted, with ~4-6 DB reads and 2 Redis reads, on every query. Found in the review of release PR #9270 (Greptile, mcp-tools.ts ~111).

Changes, all in lib/mothership/mcp-tools.ts:

Bounded fan-out. Best-effort servers go through the existing mapSettledWithConcurrency, 8 at a time (BEST_EFFORT_DISCOVERY_CONCURRENCY). Each server's 3s wait starts when that server starts. A server still queued 3s into the query (BEST_EFFORT_DISCOVERY_START_WINDOW_MS) is skipped without starting, and reported as a DeadlineExceededError failure, which catalog.ts already logs at info. Query latency therefore stays around 6s however many servers there are, and one query leaves at most 8 slow attempts running.
Per-requester negative memo. A process-local map keyed workspaceId:serverId:userId holds a best-effort server that failed or outlived its wait. That requester skips it for 60s, and the stored error is reported again. When a timed-out discovery later succeeds, the entry is cleared, so the next query is served from the cache that discovery warmed. When it later fails, the entry's 60s restarts. The map holds at most 1000 entries and evicts the oldest. It writes no shared state: the best-effort path stays free of side effects on shared status, cooldown, DB and Redis.
Named (complete) servers are unchanged: they all start at once and are waited on until they finish.

Why the memo lives in mcp-tools rather than McpService: the 3s wait, the start window and the concurrency limit all belong to this module. McpService's bestEffort only means "write no shared state". A timeout is something only the caller sees, and a memo in the service could not tell "outlived the wait" apart from "still in flight". Joins of an in-flight attempt are already deduped by the service's inflightServerDiscovery. The memo also skips the use case's per-query authorization and DB reads, which come before that dedupe.

Type of Change

Testing

I wrote the new tests in lib/mothership/mcp-tools.test.ts (fake timers, mocked discovery) before the fix and ran them against the old code. Five of the six failed there, and all of them pass with the fix:

starts at most eight best-effort discoveries at once and skips the rest once the start window closes: fails before the fix (all 20 start)
keeps discovering fast servers while slow ones hold their slots: guards against batching (passes before and after)
measures each server's wait from its own start, not the query's: fails before the fix
lists every named server at once and waits for each however long it takes: fails before the fix (named servers stay unbounded; best-effort servers are capped next to them)
does not reattempt a failed best-effort server for the same requester until the memo expires: fails before the fix (also checks that another requester, and the same requester after the TTL, do retry)
skips a server whose discovery outlived the wait until that discovery settles, then serves it: fails before the fix

Each guard was reverted on its own and a test went red every time: no cap, no start window, wait counted from the query's start, no memo check, a late success that leaves the memo in place, and timeouts left out of the memo.

The touched and dependent suites pass: mcp-tools, catalog-registry, process-contents and integrations/catalog/route (106 tests). apps/sim type-check and biome are clean.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…down server every query Broad chat catalog discovery started best-effort discovery for every usable workspace server at once. The 3s wait only ends the caller's wait, so each attempt kept its sockets open for up to ~2 min, and a workspace with many servers could burst past the connection pool and evict other users' warm connections. Best-effort failures write no shared cooldown, so a down server was reattempted (with its DB and Redis reads) on every query. - Run best-effort discovery through mapSettledWithConcurrency with 8 at once; each server's 3s wait counts from its own start, and servers still queued 3s into the query are skipped unstarted, bounding query latency at ~6s. - Remember, per process and per requester (workspace:server:user, 60s, capped at 1000 entries), a best-effort server that failed or outlived its wait, and skip it until the memo expires or the late discovery succeeds. Nothing is written to shared state. - Named servers are unchanged: listed at once, waited on to completion.

vercel · 2026-10-21T04:08:00Z

greptile-apps · 2026-10-21T04:11:30Z

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Bounds broad MCP discovery and remembers failures for each requester.

Chat catalog lookups start optional MCP servers in bounded batches.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Discover MCP tools] --> B[Named servers]
  A --> C[Best-effort servers]
  B --> D[Start all and wait until done]
  C --> E{Recent failure?}
  E -->|Yes| F[Return stored failure]
  E -->|No| G{Start window open?}
  G -->|No| H[Skip server]
  G -->|Yes| I[Discover with eight workers]
  I --> J{Done within three seconds?}
  J -->|Yes| K[Return tools or remember failure]
  J -->|No| L[Remember timeout and continue discovery]
  L --> M[Update cache only if its entry still matches]

_{Reviews (2) · Last reviewed commit: "fix(mothership): keep the best-effort me..." · Reviewed by Greptile}

cubic-dev-ai

All reported issues were addressed across 2 files

_{Heads up: you’re close to your flex budget. Increase your flex budget so reviews don’t pause.

Turn on auto-fix | Re-trigger cubic}

…its own attempt settle it - Replace the hand-rolled TTL map with LRUCache (max 1000, ttl 60s), per the repo's caching rule. - A timed-out attempt's late outcome now settles the memo only while the entry is still that attempt's: an older attempt's late success no longer erases a newer attempt's failure, and its late failure no longer recreates a memo after a newer attempt succeeded.

waleedlatif1 · 2026-10-21T04:23:34Z

@greptile

waleedlatif1 · 2026-10-21T04:23:35Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-21T04:23:41Z

cubic-dev-ai

No issues found across 2 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Heads up: you’re close to your flex budget. Increase your flex budget so reviews don’t pause.

Turn on auto-fix | Re-trigger cubic}

greptile-apps Bot reviewed Oct 21, 2026

View reviewed changes

Comment thread apps/sim/lib/mothership/mcp-tools.ts Outdated

cubic-dev-ai Bot reviewed Oct 21, 2026

View reviewed changes

Comment thread apps/sim/lib/mothership/mcp-tools.ts Outdated

vercel Bot temporarily deployed to Preview October 21, 2026 04:21 Inactive

cubic-dev-ai Bot reviewed Oct 21, 2026

View reviewed changes

waleedlatif1 merged commit 5d029a3 into staging Oct 21, 2026
44 checks passed

waleedlatif1 deleted the fix/bound-best-effort-mcp-discovery branch October 21, 2026 04:29

waleedlatif1 mentioned this pull request Oct 21, 2026

v0.9.30: mship improvements, mcp tool and skill discovery, docs refinements #9270

Merged

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

fix(mothership): bound best-effort mcp discovery and stop retrying a down server every query - #9273

fix(mothership): bound best-effort mcp discovery and stop retrying a down server every query#9273
waleedlatif1 merged 2 commits into
stagingfrom
fix/bound-best-effort-mcp-discovery

waleedlatif1 commented Oct 21, 2026

Uh oh!

vercel Bot commented Oct 21, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 21, 2026 •

edited

Loading

Diagram

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

waleedlatif1 commented Oct 21, 2026

Uh oh!

waleedlatif1 commented Oct 21, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 21, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

waleedlatif1 commented Oct 21, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Diagram

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

waleedlatif1 commented Oct 21, 2026

Uh oh!

waleedlatif1 commented Oct 21, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 21, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Oct 21, 2026 •

edited

Loading

greptile-apps Bot commented Oct 21, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading