refactor(mcp): move builtin server knowledge out of the MCP runtime

[DeJeune]

Branch strategy

• Active development targets main.

What this PR does

Before this PR:

McpRuntimeService.getOrCreateClient was 470 lines / 47 branch points and knew individual built-in servers by name: @cherry/flomo and @cherry/nowledge-mem were stored as inMemory rows but hard-coded to HTTP URLs inside the transport switch, @cherry/mcp-auto-install was excluded from the in-memory branch by name, and its MCP_REGISTRY_PATH was set inline in the npx branch.

After this PR:

The built-in catalog moved to @shared/data/presets/mcpServers.ts and declares those two servers as streamableHttp with their real baseUrl; a new BuiltinMcpServerSeeder migrates already-installed rows (and legacy inMemory mcp-auto-install rows to stdio). With the data honest, the runtime needs no name knowledge at all, so getOrCreateClient was split into reuseLiveClient / connectClient / connectWithFallback / finishOAuth, with transport construction in mcpTransport.ts, npx/uvx/uv resolution as a runner table in mcpLaunch.ts, and the SDK loader plus transport policy in mcpClientSdk.ts.

Fixes # None

Why we need it and why it was done in this way

The following tradeoffs were made:

• The seeder only rewrites rows still stored as inMemory, so re-running is a no-op, later user edits survive, and a built-in the user never installed (or deleted) is never resurrected — at the cost of not reconciling anything else.
• Migrating flomo to a real baseUrl changes its OAuth storage key (md5(baseUrl)), so affected users re-authorize once. Preserving the old token was rejected because md5('') is shared by every server without a baseUrl (flomo and nowledge-mem collide on it today), and a wrong token simply triggers the existing 401 → re-auth path.

The following alternatives were considered:

• Keeping the URL table in the runtime behind a resolver function — rejected, it leaves two sources of truth for the same endpoint.
• A hand-written Drizzle data migration instead of a seeder — rejected, the seeding guide reserves the migration chain for schema and points one-time data fixes for existing users at a run-on-change seeder with an idempotent guard.

Links to places where the discussion took place:

None

Breaking changes

In Settings → MCP, @cherry/flomo and @cherry/nowledge-mem now show as Streamable HTTP with a URL instead of in-memory, and a user who had authorized flomo is asked to authorize once more on the next connection. Recorded in v2-refactor-temp/docs/breaking-changes/2026-08-15-builtin-mcp-servers-http-transport.md.

Special notes for your reviewer

• Two behavior fixes ride along: transport fallback (405/404 → other transport) now also applies to built-in HTTP servers, and the bun argument rewrite checks position instead of membership (args.includes('x') previously launched bun -y pkg x when a package argument happened to be x or -y).
• Deliberately unchanged: the OAuth serverUrlHash and getServerKey still read the stored row, and resolveMcpConfigTransportType in McpServerFields.tsx is now redundant but left alone.
• Not covered by tests: a manual smoke run against a real flomo / npx stdio / in-memory server.

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

• Branch: This PR targets main
• PR: The PR description is expressive enough and will help future contributors
• Code: Write code that humans can understand and Keep it simple
• Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
• Upgrade: Impact of this change on upgrade flows was considered and addressed if required
• Documentation: A user-guide update was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior.
• Self-review: I have reviewed my own code (e.g., via /gh-pr-review, gh pr diff, or GitHub UI) before requesting review from others

Release note

The built-in flomo and Nowledge Mem MCP servers now connect over Streamable HTTP with their real endpoint instead of an in-memory transport; installed servers are migrated automatically. action required: if you had authorized flomo, authorize it once more on the next connection.

[claude]

Code review complete. No high-confidence issues caused or made reachable by this PR were found.

Verified areas:

• Runtime refactor equivalence (client reuse / fallback loop / OAuth reconnect / pendingClients lifecycle) against the pre-refactor getOrCreateClient.
• Transport dispatch change from name-aware (isInMemoryBuiltinMcpServer) to type-only (server.type === 'inMemory'): a non-builtin inMemory+command row would now throw Unknown in-memory MCP server instead of running as stdio, but no row producer reaches that state — the form gates inMemory behind !isBuiltin, JSON import rejects non-builtin inMemory via the schema refine, and protocol/DXT/Npx only emit stdio/sse/streamableHttp.
• BuiltinMcpServerSeeder idempotency (WHERE type='inMemory' makes re-runs no-ops once migrated), the mcp-auto-install args-clobber path being pre-empted by the form's resolveMcpConfigTransportType self-heal, headers-merge equivalence, and preset/DTO/DB field completeness.

CI, signatures, and merge readiness are outside the scope of this review.

[zhangjiadi225]

The overall direction is sound: moving the preset catalog into shared data and separating transport/launch concerns removes static builtin endpoint knowledge from the runtime. However, the refactor currently introduces a first-connect single-flight race, leaves legacy builtin identity incomplete during seeding, and carries forward an unregistered main-process filesystem path. These should be addressed before merge.

[kangfenmao]

The transport and preset refactor is moving in a good direction, and the existing review already covers the first-connect single-flight race, legacy builtin identity, and path registration. Two additional correctness gaps remain: a fresh QVeris install has no UI path to provide the credential that activation still requires after the transport change, and case-sensitive Authorization detection can mix static credentials with the OAuth provider. Please address these before merge.

[claude]

The review is complete. No findings to report — the PR correctly addresses all previous feedback and the refactoring is well-structured with comprehensive test coverage.

[zhangjiadi225]

LGTM. The previously requested changes are addressed.

[cherry-ai-bot]

Cherry Review · 阻塞

1 blocker · 2 warning · 2 notice
逐条见行内评论。

[claude]

🤖 Generated with Claude Code

Review Summary

I've completed a thorough review of this refactoring PR. The implementation is well-designed with correct handling of:

• Seeder scope: Only migrates inMemory rows owned by builtin (installSource = 'builtin' or NULL), preserves user edits, and never installs new servers
• Concurrent client creation: Single-flight pattern correctly prevents client leaks with synchronous pendingClients.set() registration
• Arg transformation: Position-based check fixes the previous membership-based bug
• QVeris API key: Properly validated at connection time
• Breaking change: OAuth re-authorization is documented and intentional

No findings to report. The test coverage is comprehensive and the architectural separation into mcpTransport.ts, mcpLaunch.ts, and mcpClientSdk.ts is clean.