Repository navigation
Remove connector kind from remote connectors - #631
Merged
Merged
Conversation
Drop the kind/type dimension from remote connectors so each connector is
identified only by its user-chosen name (instanceId).
Breaking changes:
- WebSocket URL: /@username/connectors/<name> (was .../kind/name)
- Session key: [userId, instanceId] (was [userId, kind, instanceId])
- MCP caller context refs: { instanceId } only
- D1 migration drops kind column; dedupes on (user_id, instance_id)
- connector.hello connectorKind is ignored (still accepted from protocol)
Updates account UI, settings API, capability metadata, status reporting,
and docs accordingly.
📝 WalkthroughWalkthroughThis PR removes connector ChangesRemove connector kind
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant WorkerIndex
participant ConnectorSessionKey
participant RemoteConnectorClient
participant SnapshotCache
WorkerIndex->>ConnectorSessionKey: parseUserScopedConnectorRoutePath
ConnectorSessionKey-->>WorkerIndex: userId, instanceId
WorkerIndex->>RemoteConnectorClient: createRemoteConnectorMcpClient(instanceId)
RemoteConnectorClient->>SnapshotCache: getCachedRemoteConnectorSnapshot(userId, instanceId)
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 6, 2026 00:18
Contributor
|
🔎 Preview deployed: https://kody-pr-631.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/remote-connector/settings-service.ts (1)
104-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant conflict detection / extra DB round-trip.
The
nameConflictcheck (Lines 104-112) already rejects any row whoseinstance_id === instanceIdandid !== input.id, which fully covers the uniqueness rule for both create and update. In the update branchexisting.id === input.id, so therefConflictblock (Lines 129-138) re-queries byinstanceIdand re-checks the exact same condition, adding a redundant DB call and dead code. Similarly, in the create pathnameConflictthrows beforeexistingis used, so thegetRemoteConnectorSettingRowByInstanceIdbranch always resolves tonull.♻️ Proposed simplification
const existing = input.id ? await getRemoteConnectorSettingRowById({ db: input.env.APP_DB, userId: input.userId, id: input.id, }) - : await getRemoteConnectorSettingRowByInstanceId({ - db: input.env.APP_DB, - userId: input.userId, - instanceId, - }) + : null if (input.id && !existing) { throw new Error('Remote connector setting not found.') } - - if (input.id) { - const refConflict = await getRemoteConnectorSettingRowByInstanceId({ - db: input.env.APP_DB, - userId: input.userId, - instanceId, - }) - if (refConflict && refConflict.id !== existing?.id) { - throw new Error('A remote connector with this name already exists.') - } - }🤖 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/remote-connector/settings-service.ts` around lines 104 - 138, The conflict detection in settings-service’s update/create flow is duplicated: the initial nameConflict check already covers any row with the same instanceId and a different id, so the later refConflict block and its extra getRemoteConnectorSettingRowByInstanceId lookup are redundant. Remove the refConflict branch from the existing input.id path and rely on the existing nameConflict plus the existing row lookup to keep the uniqueness check in one place.
🤖 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.
Nitpick comments:
In `@packages/worker/src/remote-connector/settings-service.ts`:
- Around line 104-138: The conflict detection in settings-service’s
update/create flow is duplicated: the initial nameConflict check already covers
any row with the same instanceId and a different id, so the later refConflict
block and its extra getRemoteConnectorSettingRowByInstanceId lookup are
redundant. Remove the refConflict branch from the existing input.id path and
rely on the existing nameConflict plus the existing row lookup to keep the
uniqueness check in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fc6e598-4121-4d2d-8d60-2d02b9901ea1
📒 Files selected for processing (53)
docs/contributing/architecture/remote-connectors.mdpackages/shared/src/chat.tspackages/shared/src/remote-connectors.tspackages/worker/client/routes/account-remote-connectors.tsxpackages/worker/migrations/0050-drop-remote-connector-kind.sqlpackages/worker/src/app/account-deletion.node.test.tspackages/worker/src/app/account-deletion.tspackages/worker/src/app/account-export.node.test.tspackages/worker/src/app/account-export.tspackages/worker/src/app/account-remote-connectors-data.tspackages/worker/src/app/handlers/account-remote-connectors.node.test.tspackages/worker/src/app/handlers/account-remote-connectors.tspackages/worker/src/app/loader-data.tspackages/worker/src/index.tspackages/worker/src/mcp-auth.workers.test.tspackages/worker/src/mcp/capabilities/meta/meta-list-capabilities.node.test.tspackages/worker/src/mcp/capabilities/meta/meta-list-capabilities.tspackages/worker/src/mcp/capabilities/meta/meta-list-remote-connector-status.tspackages/worker/src/mcp/capabilities/meta/search.tspackages/worker/src/mcp/capabilities/registry.tspackages/worker/src/mcp/capabilities/remote-connector/index.tspackages/worker/src/mcp/capabilities/types.tspackages/worker/src/mcp/executor.node.test.tspackages/worker/src/mcp/index.tspackages/worker/src/mcp/kody-remote-types.tspackages/worker/src/mcp/run-kody-registry.node.test.tspackages/worker/src/mcp/run-kody-registry.tspackages/worker/src/mcp/tools/search-format.node.test.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/mcp/tools/search-handler.node.test.tspackages/worker/src/mcp/tools/search.node.test.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/package-invocations/http.workers.test.tspackages/worker/src/package-invocations/service.node.test.tspackages/worker/src/package-runtime/package-workflows.node.test.tspackages/worker/src/remote-connector/client.tspackages/worker/src/remote-connector/connector-session-key.node.test.tspackages/worker/src/remote-connector/connector-session-key.tspackages/worker/src/remote-connector/remote-connectors-shared.node.test.tspackages/worker/src/remote-connector/remote-domain-id.node.test.tspackages/worker/src/remote-connector/resolve-remote-connector-secret.node.test.tspackages/worker/src/remote-connector/resolve-remote-connector-secret.tspackages/worker/src/remote-connector/session.node.test.tspackages/worker/src/remote-connector/session.tspackages/worker/src/remote-connector/settings-repo.tspackages/worker/src/remote-connector/settings-service.node.test.tspackages/worker/src/remote-connector/settings-service.tspackages/worker/src/remote-connector/settings-types.tspackages/worker/src/remote-connector/snapshot-cache.node.test.tspackages/worker/src/remote-connector/snapshot-cache.tspackages/worker/src/remote-connector/status.tspackages/worker/src/remote-connector/types.tspackages/worker/src/security/public-route-hardening.workers.test.ts
💤 Files with no reviewable changes (20)
- packages/worker/src/remote-connector/types.ts
- packages/worker/src/mcp/kody-remote-types.ts
- packages/worker/src/remote-connector/settings-types.ts
- packages/worker/src/mcp/capabilities/meta/meta-list-capabilities.ts
- packages/worker/src/mcp/capabilities/types.ts
- packages/worker/src/remote-connector/resolve-remote-connector-secret.node.test.ts
- packages/worker/src/app/account-remote-connectors-data.ts
- packages/worker/src/app/handlers/account-remote-connectors.ts
- packages/worker/src/remote-connector/client.ts
- packages/worker/src/mcp/tools/search-format.ts
- packages/worker/src/index.ts
- packages/worker/src/mcp/tools/search.ts
- packages/worker/src/remote-connector/snapshot-cache.node.test.ts
- packages/worker/src/mcp/tools/search-format.node.test.ts
- packages/worker/src/app/loader-data.ts
- packages/shared/src/chat.ts
- packages/worker/src/app/account-export.node.test.ts
- packages/worker/src/remote-connector/snapshot-cache.ts
- packages/worker/src/remote-connector/session.node.test.ts
- packages/worker/src/mcp/capabilities/meta/search.ts
The initial nameConflict scan already enforces per-user instanceId uniqueness for both create and update paths, so drop the redundant refConflict lookup and create-path instanceId fetch.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Use an explicit MIN(rowid) join when collapsing duplicate connector settings so the surviving row's secret/metadata is deterministic. Normalize connector.hello connectorId with normalizeRemoteConnectorInstanceId so persisted IDs match routing, session keys, and saved settings.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit af399ab. Configure here.
Normalize instance_id to lowercase when rebuilding remote_connector_settings so migrated rows match runtime routing and auth, and dedupe on (user_id, lower(instance_id)).
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
2 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.
Summary
Removes the
kinddimension from remote connectors entirely. Connectors are now identified only by their user-chosen name (instanceId).This is a hard break — no backward compatibility for old URLs, session keys, or stored settings shape.
Breaking changes
wss://…/@user/connectors/lights/homewss://…/@user/connectors/home[userId, kind, instanceId][userId, instanceId]{ kind, instanceId }{ instanceId }meta_list_remote_connector_status.connector_kindconnector_idMigration
0050-drop-remote-connector-kind.sqlrebuildsremote_connector_settingswithoutkind.(user_id, lower(instance_id)), keeping the row with minimumrowidand lowercasinginstance_id.Protocol note
connector.hellostill acceptsconnectorKindfrom connector-kit clients, but Kody ignores it for routing and auth.connectorIdis normalized (trim + lowercase) on hello.Review follow-ups (addressed)
settings-service.ts(CodeRabbit)MIN(rowid)join (Bugbot)connectorIdand migrationinstance_idcasing (Bugbot)Validation
npm run validatepasses on latest commit3cdbd55cSummary by CodeRabbit
New Features
Bug Fixes
Documentation