[waleedlatif1]

Summary

Follow-up to #9263 (discover every workspace MCP server without tagging) and #9262 (BM25 ranking), fixing four audit findings.

1. Best-effort discovery wrote shared state. A server's status, failure count, cooldown and OAuth row are shared by the workspace, but credentials resolve per requester. Broad catalog calls now reach every usable server, so a member without a personal {{VAR}} made discovery fail. That stamped the server failing and set a 120s workspace-wide cooldown, which broke another member's tagged or targeted discovery and call re-resolution (McpServerCooldownError). For an OAuth server nobody connected, it inserted an empty OAuth row and marked the server disconnected for everyone.
   • McpService.discoverServerTools has a bestEffort mode, used only for untagged broad discovery (through discoverMcpServerToolsUseCase from buildMcpToolSchemas). It only reads shared health state. On failure it writes no status, failure count, cooldown or cache eviction. On success it only warms the tool cache. It has its own in-flight key.
   • Before reading the cache or connecting, it skips a server the requester can't authenticate to: a referenced variable they have no value for (new findMissingMcpConfigEnvVars, which records no secret usage), or an OAuth server with no authorized tokens (loadOauthRow, which never creates a row). So another member's cached tools are never served to a requester who lacks the credentials to call them.
   • Tagged and targeted discovery keep today's health bookkeeping.
2. listUsableMcpServerIds errors failed the whole catalog, built-ins included, which undid fix(mothership): keep integration discovery working when a tagged mcp server fails or lists no tools #9260. Broad requests now log the error and keep built-ins and tagged servers. Targeted MCP requests (service: mcp:<id> / exact MCP tool id) still fail with the error.
3. BM25 recall regression. A query with no content terms after tokenization (all, any, what can I use, a non-ASCII query) returned total: 0, so the agent could conclude there are no integrations. It now returns the unranked compact listing, as an empty query does. Exact-ID ranking is unchanged.
4. A tagged server that's no longer usable (disabled, deleted, Credential Group, other workspace) was dropped from broad requests without a log. It's logged again.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• Other: ___________

Testing

Regression tests. With the source changes reverted and only the new tests kept, 9 of them fail:

• lib/mcp/service.test.ts › McpService best-effort discovery:
  • a failing best-effort discovery leaves status and cooldown alone, so another member's targeted discovery still connects
  • a requester missing a variable is skipped without connecting, even when another member left the tools cached, and that member is still served
  • an unauthorized OAuth server is skipped without getOrCreateOauthRow or a pending stamp
  • a successful best-effort discovery warms the cache without stamping status
• lib/mcp/resolve-config.test.ts › findMissingMcpConfigEnvVars
• catalog.test.ts:
  • lists every operation, compactly, for a query with no content terms
  • keeps built-ins and tagged servers when the workspace server list fails
  • the untagged-discovery wiring tests now assert bestEffort: true

With the fix, all 8 touched or adjacent suites pass: 128 tests across service, service-pool, resolve-config, use-cases, execute-tool, mcp-tools, catalog and the catalog route. bun run type-check in apps/sim is clean.

Reviewers: the OAuth row is one per server (loadOauthRow keys on mcpServerId), so "can authenticate" for OAuth means that row holds tokens.

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)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Oct 21, 2026 3:17am UTC

[cubic-dev-ai]

2 issues found across 8 files

Confidence score: 3/5

• In service.ts, loadOauthRow clears and updates the shared OAuth row when decryption fails, so broad best-effort discovery can mutate OAuth state despite being documented as side-effect-free. Keep this path read-only on decrypt failure.
• In resolve-config.ts, decrypt failures become "", but the availability check only rejects undefined, so discovery can attempt a connection with unusable credentials. Treat failed decryption as unavailable.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/lib/mcp/service.ts">

<violation number="1" location="apps/sim/lib/mcp/service.ts:1329">
P2: This best-effort path is documented as side-effect-free, but `loadOauthRow` clears and updates the shared OAuth row when token decryption fails. A key-rotation or corrupted-token encounter during broad discovery can therefore erase the stored grant and force reauthorization for every workspace member; use a non-mutating token-presence read for this gate.</violation>
</file>

<file name="apps/sim/lib/mcp/resolve-config.ts">

<violation number="1" location="apps/sim/lib/mcp/resolve-config.ts:168">
P2: A failed secret decryption is represented as an empty string, but this helper treats that key as available because the resolver only checks for `undefined`. Best-effort discovery can therefore connect with an unusable credential instead of skipping the server; exclude failed effective keys before resolving.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic</sub>

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The changes appear safe to merge.

Summary

This PR keeps broad MCP discovery from changing shared server health and makes catalog results more resilient.

• Broad MCP discovery no longer disturbs shared server state.
• Broad catalogs keep built-in tools when MCP server listing fails.
• Searches with no content words show the compact integration list.

The earlier empty-value finding is fixed. No new actionable issues were found in the changes since the previous review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Catalog request] --> B{Server explicitly requested?}
  B -->|Yes| C[Tracked discovery]
  B -->|No| D[Best-effort discovery]
  D --> E{Requester has credentials?}
  E -->|No| F[Skip server]
  E -->|Yes| G[Read cache or connect]
  G --> H[Keep shared health unchanged]
  G --> I[Preserve OAuth grant]
  C --> J[Keep existing health bookkeeping]

Loading

_{Reviews (3) · Last reviewed commit: "fix(mothership): keep the shared mcp oau..." · Reviewed by Greptile}

[waleedlatif1]

@greptile

[waleedlatif1]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

All reported issues were addressed across 12 files

<sub>Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic</sub>

[waleedlatif1]

@greptile

[waleedlatif1]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

No issues found across 14 files

Confidence score: 5/5

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

_{Turn on auto-fix | Re-trigger cubic}