Repository navigation
Add MCP client support: connect Kody to user-added remote MCP servers - #662
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds user-managed remote MCP client servers end to end: shared validation and ID helpers, persistent per-user hub/runtime support, capability synthesis and execution wiring, account UI/API routes, deletion cleanup, and documentation. ChangesMCP Client Servers Feature
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AccountMcpServersRoute
participant AccountMcpServersApiHandler
participant SettingsService
participant McpClientHub
AccountMcpServersRoute->>AccountMcpServersApiHandler: POST { action: "add", name, url }
AccountMcpServersApiHandler->>SettingsService: addMcpServer(userId, name, url, baseUrl)
SettingsService->>McpClientHub: addServer(...)
McpClientHub-->>SettingsService: connectResult
SettingsService-->>AccountMcpServersApiHandler: setting + connection
AccountMcpServersApiHandler-->>AccountMcpServersRoute: JSON { servers, selectedServerId }
sequenceDiagram
participant GeneratedCode
participant PackageAppProxy
participant RuntimeBridge
participant McpClientHub
participant RemoteServer
GeneratedCode->>PackageAppProxy: kody.mcp["linear"].create_issue(args)
PackageAppProxy->>RuntimeBridge: callCapability("mcp:linear:create_issue", args)
RuntimeBridge->>McpClientHub: callTool(serverId, toolName, args)
McpClientHub->>RemoteServer: forward tool call
RemoteServer-->>McpClientHub: CallToolResult
RuntimeBridge-->>GeneratedCode: structuredContent / content
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kentcdodds
marked this pull request as ready for review
July 8, 2026 03:55
Contributor
|
🔎 Preview deployed: https://kody-pr-662.kody-a99.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/worker/src/mcp/tools/search.ts (1)
298-305: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAccessor-building logic duplicates
buildNamespacedKodyAccessorfromsearch-format.ts.The regex check and
JSON.stringifyaccessor construction formcp-servermirrors the same pattern already duplicated forremote-connector(lines 290-296). Consider exportingbuildNamespacedKodyAccessorfromsearch-format.tsand reusing it here to keep accessor generation in one place.♻️ Optional refactor
In
search-format.ts, export the helper:-export function buildNamespacedKodyAccessor(input: { +export function buildNamespacedKodyAccessor(input: {Then in
search.ts, replace the manual accessor construction:if (topMatch.source === 'mcp-server' && topMatch.mcpServer) { - const serverName = topMatch.mcpServer.kodyName - const toolName = topMatch.mcpServer.toolName - const accessor = /^[A-Za-z_$][\w$]*$/.test(toolName) - ? `kody.mcp[${JSON.stringify(serverName)}].${toolName}` - : `kody.mcp[${JSON.stringify(serverName)}][${JSON.stringify(toolName)}]` + const accessor = buildNamespacedKodyAccessor({ + namespace: 'mcp', + entryName: topMatch.mcpServer.kodyName, + toolName: topMatch.mcpServer.toolName, + }) return `Inspect capability detail with \`search({ entity: "${topMatch.name}:capability" })\` to confirm the TypeScript call shape, then call it from \`execute\` via \`${accessor}(args)\`.` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/tools/search.ts` around lines 298 - 305, The accessor construction in the mcp-server branch of search formatting is duplicating the same namespaced accessor logic already handled by buildNamespacedKodyAccessor in search-format.ts. Export and reuse buildNamespacedKodyAccessor from search-format.ts, then update search.ts to call that helper for the mcp-server path instead of rebuilding the regex and JSON.stringify accessor inline, keeping accessor generation centralized alongside the existing remote-connector usage.packages/worker/src/mcp/capabilities/mcp-server/index.ts (1)
85-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting a helper for repeated annotation access.
The same
(tool.annotations as Record<string, unknown> | undefined)?.['...']pattern is used three times forreadOnlyHint,idempotentHint, anddestructiveHint. A small helper likegetAnnotationFlag(tool, key)would reduce duplication and the repeated cast.♻️ Optional refactor
+function getAnnotationFlag( + tool: McpServerToolDescriptor, + key: string, +): boolean { + return Boolean( + (tool.annotations as Record<string, unknown> | undefined)?.[key], + ) +} + function createCapabilityFromTool(input: { // ... }) { // ... readOnly: getAnnotationFlag(tool, 'readOnlyHint'), idempotent: getAnnotationFlag(tool, 'idempotentHint'), destructive: getAnnotationFlag(tool, 'destructiveHint'),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/mcp-server/index.ts` around lines 85 - 99, The annotation lookup in the MCP server tool capability mapping repeats the same cast-and-index pattern for readOnlyHint, idempotentHint, and destructiveHint. Extract a small helper such as getAnnotationFlag(tool, key) near the mapping logic in mcp-server/index.ts, and use it for the readOnly, idempotent, and destructive fields to centralize the annotations access and remove duplication.packages/worker/src/app/handlers/account-mcp-servers.node.test.ts (1)
142-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
reconnectandrefreshactions.The test suite covers
add,set-enabled,delete, and OAuth callback, but does not exercise thereconnectorrefreshactions. These actions involverequireSettingownership checks and hub client calls (reconnectServer/refreshServer) that should be verified. The mock forcreateMcpClientHubClientalso only includeshandleOAuthCallback— addingreconnectServerandrefreshServermocks would be needed.Additionally, the add-action test (lines 175–206) only asserts
okandselectedServerIdon the response payload. It does not verify theserversarray or whether theauthUrlfrom theconnectionresult is available to the client. Consider asserting the full response shape to catch regressions in the add flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-mcp-servers.node.test.ts` around lines 142 - 281, The MCP servers handler tests are missing coverage for the reconnect and refresh actions, and the add-action assertion is too narrow. Extend the mock returned by createMcpClientHubClient to include reconnectServer and refreshServer, then add tests in account-mcp-servers.node.test.ts that exercise the createAccountMcpServersApiHandler POST paths for reconnect and refresh, verifying requireSetting ownership checks and the expected hub client calls. Also strengthen the add-action test around createAccountMcpServersApiHandler by asserting the full response payload from the add flow, including the servers array and any authUrl returned from the connection result, so regressions in the response shape are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp-client/settings-service.ts`:
- Around line 114-134: The MCP server setup flow leaves an orphaned Durable
Object registration if the D1 persistence step fails after hub.addServer
succeeds. Update the add-server path in settings-service.ts so
insertMcpServerSettingRow is wrapped in its own try/catch after the
hub.addServer call, and on failure call hub.removeServer using the
connection/server identifier before rethrowing. Keep the cleanup local to the
existing connection setup logic so the hub.addServer and
insertMcpServerSettingRow sequence stays easy to follow.
In `@packages/worker/src/mcp/capabilities/mcp-server/index.ts`:
- Around line 128-142: The catch block in MCP server tool execution is
overwriting the original `hub.callTool` failure if `getMcpServerStatus` throws.
Update the `try/catch` around the `hub.callTool` path in `McpServerCapability`
so the status lookup is wrapped in its own `try/catch`, and if status retrieval
fails, fall back to rethrowing the original tool error with the existing
`ref.name`, `tool.name`, and error-message formatting logic.
---
Nitpick comments:
In `@packages/worker/src/app/handlers/account-mcp-servers.node.test.ts`:
- Around line 142-281: The MCP servers handler tests are missing coverage for
the reconnect and refresh actions, and the add-action assertion is too narrow.
Extend the mock returned by createMcpClientHubClient to include reconnectServer
and refreshServer, then add tests in account-mcp-servers.node.test.ts that
exercise the createAccountMcpServersApiHandler POST paths for reconnect and
refresh, verifying requireSetting ownership checks and the expected hub client
calls. Also strengthen the add-action test around
createAccountMcpServersApiHandler by asserting the full response payload from
the add flow, including the servers array and any authUrl returned from the
connection result, so regressions in the response shape are caught.
In `@packages/worker/src/mcp/capabilities/mcp-server/index.ts`:
- Around line 85-99: The annotation lookup in the MCP server tool capability
mapping repeats the same cast-and-index pattern for readOnlyHint,
idempotentHint, and destructiveHint. Extract a small helper such as
getAnnotationFlag(tool, key) near the mapping logic in mcp-server/index.ts, and
use it for the readOnly, idempotent, and destructive fields to centralize the
annotations access and remove duplication.
In `@packages/worker/src/mcp/tools/search.ts`:
- Around line 298-305: The accessor construction in the mcp-server branch of
search formatting is duplicating the same namespaced accessor logic already
handled by buildNamespacedKodyAccessor in search-format.ts. Export and reuse
buildNamespacedKodyAccessor from search-format.ts, then update search.ts to call
that helper for the mcp-server path instead of rebuilding the regex and
JSON.stringify accessor inline, keeping accessor generation centralized
alongside the existing remote-connector usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae470f11-b3ee-4648-b3a1-7d7dfc1234bf
📒 Files selected for processing (62)
docs/contributing/adding-capabilities.mddocs/contributing/architecture/index.mddocs/contributing/architecture/mcp-client-servers.mddocs/contributing/architecture/primitives.yamlpackages/shared/src/mcp-servers.node.test.tspackages/shared/src/mcp-servers.tspackages/worker/client/app.tsxpackages/worker/client/routes/account-mcp-servers.tsxpackages/worker/client/routes/account.tsxpackages/worker/client/routes/index.tsxpackages/worker/migrations/0053-mcp-server-settings.sqlpackages/worker/src/app/account-data-targets.tspackages/worker/src/app/account-deletion.tspackages/worker/src/app/account-mcp-servers-data.tspackages/worker/src/app/handler.node.test.tspackages/worker/src/app/handlers/account-mcp-servers.node.test.tspackages/worker/src/app/handlers/account-mcp-servers.tspackages/worker/src/app/loader-data.tspackages/worker/src/app/router.tspackages/worker/src/app/routes.tspackages/worker/src/app/ssr-render.node.test.tspackages/worker/src/env-schema.tspackages/worker/src/execute-maintenance.node.test.tspackages/worker/src/execute-maintenance.tspackages/worker/src/index.tspackages/worker/src/mcp-client/hub-client.tspackages/worker/src/mcp-client/hub.tspackages/worker/src/mcp-client/mcp-domain-id.node.test.tspackages/worker/src/mcp-client/mcp-domain-id.tspackages/worker/src/mcp-client/settings-repo.tspackages/worker/src/mcp-client/settings-service.tspackages/worker/src/mcp-client/settings-types.tspackages/worker/src/mcp-client/status.tspackages/worker/src/mcp-client/types.tspackages/worker/src/mcp/capabilities/build-capability-registry.tspackages/worker/src/mcp/capabilities/builtin-domains.tspackages/worker/src/mcp/capabilities/define-capability.tspackages/worker/src/mcp/capabilities/domain-metadata.tspackages/worker/src/mcp/capabilities/mcp-server/index.tspackages/worker/src/mcp/capabilities/mcp-server/synthesize.node.test.tspackages/worker/src/mcp/capabilities/mcp-servers/domain.tspackages/worker/src/mcp/capabilities/mcp-servers/mcp-server-add.tspackages/worker/src/mcp/capabilities/mcp-servers/mcp-server-list.tspackages/worker/src/mcp/capabilities/mcp-servers/mcp-server-reconnect.tspackages/worker/src/mcp/capabilities/mcp-servers/mcp-server-refresh.tspackages/worker/src/mcp/capabilities/mcp-servers/mcp-server-remove.tspackages/worker/src/mcp/capabilities/mcp-servers/mcp-server-set-enabled.tspackages/worker/src/mcp/capabilities/mcp-servers/shared.tspackages/worker/src/mcp/capabilities/meta/meta-list-capabilities.tspackages/worker/src/mcp/capabilities/registry.tspackages/worker/src/mcp/capabilities/types.tspackages/worker/src/mcp/executor.tspackages/worker/src/mcp/kody-remote-proxy-source.tspackages/worker/src/mcp/kody-remote-types.tspackages/worker/src/mcp/run-kody-registry.tspackages/worker/src/mcp/server-instructions.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/oauth-handlers.workers.test.tspackages/worker/src/package-runtime/package-app.tspackages/worker/worker-configuration.d.tspackages/worker/wrangler.jsonc
| await this.manager.waitForConnections({ | ||
| timeout: connectionSettleTimeoutMs, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
OAuth path skips tool discovery
Medium Severity
After a successful MCP OAuth callback, the hub calls establishConnection and waitForConnections but never runs discoverIfConnected, unlike addServer and successful reconnectServer paths. If discovery is not finished when waiting ends, the server can stay non-ready with no tools, so synthesized kody.mcp capabilities and the account UI may show authorization success without callable tools until reconnect or refresh.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4569b5e. Configure here.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 72651ea. Configure here.
| const state = this.connectionStateFor(input.serverId) | ||
| if (state === 'disconnected') { | ||
| throw new Error(`MCP server "${input.serverId}" is not registered.`) | ||
| } |
Contributor
There was a problem hiding this comment.
Reconnect fails after hub-only removal
Medium Severity
deleteMcpServer removes the hub registration before the D1 row. If the database delete fails, settings still list the server but the hub no longer registers it. reconnectServer then treats that as “not registered” and errors instead of re-registering from saved URL and OAuth callback settings.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 72651ea. Configure here.
This was referenced Jul 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Kody can now act as an MCP client: users add remote MCP servers (OAuth-protected or open), and the tools those servers expose become Kody capabilities callable from execute via
kody.mcp["server-name"].tool_name(...). Servers are manageable both in the UI (/account/mcp-servers) and through the newmcp_serverscapability domain.Demo
Adding an OAuth-protected MCP server in the UI, authorizing it, and seeing its tools discovered:
Adding, connecting, and managing a plain (no-auth) MCP server:
What's included
McpClientHubDurable Object (one per user) owning the Cloudflare Agents SDKMCPClientManager; server registrations, OAuth client registrations, and tokens persist in DO SQLite storage. The hub constructs aDurableObjectOAuthClientProviderper server registration so OAuth-protected servers surface an authorization URL (the SDK'sregisterServerdoes not create one on its own).mcp_server_settingstable (migration0053) for user-scoped metadata (name, url, enabled) so the registry can list servers without waking the DO; per-user snapshot cache (30s TTL) in front of the DO.authenticatingwith anauthUrl; the browser callback at/account/mcp-servers/oauth/callbackis authenticated via the session cookie and forwarded to that user's hub, which completes the code exchange and connects. Remote-only: server URLs must be https (http allowed for loopback in local dev).mcp:<server>domains with one capability per tool (mcp:<server>:<tool>,source: 'mcp-server'), discoverable viasearchwith exactkody.mcp[...]accessors.kody.mcpproxy (mirroringkody.remote) in the execute runtime and package app runtime, with helpful errors for unknown servers/tools and disconnected servers.mcp_server_add,mcp_server_list,mcp_server_reconnect,mcp_server_refresh,mcp_server_remove,mcp_server_set_enabled./account/mcp-serverspage (add form, live status, authorize link, discovered tools, reconnect/refresh/enable/disable/remove) linked from the account page and top nav.mcp_server_settingscovered in account export and deletion; the hub DO is purged on account deletion.docs/contributing/architecture/mcp-client-servers.md; primitives map updated.Testing
npm run validatepasses (format, lint, typecheck, 611 unit tests, Playwright E2E, MCP E2E).mcp_server_addcapability; tools discovered;kody.mcp["test-tools"].add_numbers({a:20,b:22})returns{sum: 42}through the real/mcpexecute path; unknown-server error path returns a helpful message.authenticatingwith authorize link → browser authorization → callback →connectedwith tools discovered →kody.mcp["oauth-tools"].secret_number({})returns{secret: 1234}through execute./mcpend-to-end check: kody_mcp_execute_e2e.logSystem recap — adds a new primitive (high risk)
Mode: recap · Base:
main@8fa6526d· Head:13c44f07Classification: adds — introduces the
mcp-client-serversprimitive (per-user MCP client hub DO + D1 settings + synthesizedkody.mcpdomains).primitives.yamlis updated in this PR.Primitives touched
mcp-client-serversmcp_serversdomain)capability-registrymcp:<server>domainscapabilities-executekody.mcpproxy besidekody.remoteapp-ui/account/mcp-serversroutes + OAuth callback routepackage-runtimekodyproxy gainskody.mcpd1-app-dbmcp_server_settingstable (migration0053)account-exportSystem map
Change flow
sequenceDiagram participant U as User (browser) participant W as Worker (app routes) participant H as McpClientHub DO (per user) participant S as Remote MCP server U->>W: POST add { name, url } W->>H: addServer(callbackUrl) H->>S: connect (Agents SDK MCPClientManager) S-->>H: 401 + OAuth metadata H-->>U: state authenticating + authUrl U->>S: authorize in browser S->>W: GET /account/mcp-servers/oauth/callback?code&state W->>H: handleOAuthCallback(url) (session-cookie scoped) H->>S: token exchange + connect + discover tools W-->>U: 303 /account/mcp-servers?auth=successInvariants
userId;mcp_server_settingsreads/writes filter byuser_id; the OAuth callback resolves the hub through the authenticated session cookie, so state lookups never cross users; registry synthesis only reads the calling user's servers.Summary by CodeRabbit
kody.mcp[...]tooling andmcp_serverscapabilities.