[codex] Add scoped MCP toolkits#1145
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 3f68c6a | Commit Preview URL Branch Preview URL |
Jun 26 2026, 10:03 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 3f68c6a | Jun 26 2026, 10:04 PM |
Cloudflare previewTorn down — the PR is closed. |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
@executor-js/codemode-core
@executor-js/runtime-quickjs
executor
commit: |
…-mcp # Conflicts: # apps/local/src/serve.ts # apps/local/vite.config.ts
Greptile SummaryThis PR introduces scoped MCP toolkits: each toolkit gets its own
Confidence Score: 4/5Safe to merge; the core resource-binding, auth, and policy-enforcement paths are well-tested. Two non-blocking concerns exist in the toolkit server's legacy-policy heuristic and connection-list query amplification. The isLegacyConnectionPolicy heuristic can silently skip an intentional approve policy, and connectionsList with an active toolkit provider triggers O(3xN_tools) DB queries. Neither breaks existing flows, but the heuristic can produce wrong policy outcomes for certain toolkit configurations. packages/plugins/toolkits/src/server.ts (isLegacyConnectionPolicy heuristic and its interaction with connectionsList in executor.ts) Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant Envelope as MCP Envelope
participant Auth as McpAuthProvider
participant Store as McpSessionStore
participant DO as Session DO
participant Exec as Executor
Client->>Envelope: POST /mcp/toolkits/:slug
Envelope->>Auth: authenticate(request)
Auth-->>Envelope: Authenticated(principal)
Envelope->>Store: dispatch resource toolkit slug
Store->>DO: createSession(token, resource)
DO->>Exec: createExecutorHandle activeToolkitSlug
Note over Exec: toolPolicyProvider replaces global tool_policy
Exec-->>DO: ExecutorHandle
DO-->>Client: 200 OK + mcp-session-id
Client->>Envelope: POST /mcp/toolkits/:slug + mcp-session-id
Envelope->>Store: dispatch existing session
Store->>DO: forward + verify resource key matches
DO-->>Client: MCP response
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant Envelope as MCP Envelope
participant Auth as McpAuthProvider
participant Store as McpSessionStore
participant DO as Session DO
participant Exec as Executor
Client->>Envelope: POST /mcp/toolkits/:slug
Envelope->>Auth: authenticate(request)
Auth-->>Envelope: Authenticated(principal)
Envelope->>Store: dispatch resource toolkit slug
Store->>DO: createSession(token, resource)
DO->>Exec: createExecutorHandle activeToolkitSlug
Note over Exec: toolPolicyProvider replaces global tool_policy
Exec-->>DO: ExecutorHandle
DO-->>Client: 200 OK + mcp-session-id
Client->>Envelope: POST /mcp/toolkits/:slug + mcp-session-id
Envelope->>Store: dispatch existing session
Store->>DO: forward + verify resource key matches
DO-->>Client: MCP response
Reviews (9): Last reviewed commit: "Fix console route contract for nested to..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const matches = | ||
| accountId === sessionMeta.userId && organizationId === sessionMeta.organizationId; | ||
| accountId === sessionMeta.userId && | ||
| organizationId === sessionMeta.organizationId && | ||
| resourceKey === mcpResourceKey(sessionMeta.resource); |
There was a problem hiding this comment.
resource on pre-deploy DO sessions causes TypeError
sessionMeta.resource will be undefined for any Durable Object session that was created before this PR is deployed, because the stored JSON won't contain the resource field. When such a session receives a request under the new code, mcpResourceKey(sessionMeta.resource) immediately throws TypeError: Cannot read properties of undefined (reading 'kind') — both for warm DOs (where this.sessionMeta was populated by old init() code) and cold DOs (where stored JSON is deserialized without resource).
A safe fix: mcpResourceKey(sessionMeta.resource ?? defaultMcpResource). The same default applies to the resolveAndStoreSessionMeta result for the cloud (apps/cloud/src/mcp/session-durable-object.ts) and the Cloudflare host (apps/host-cloudflare/src/mcp/session-durable-object.ts).
| readonly name?: unknown; | ||
| }>(); | ||
| return new Set( | ||
| result.results | ||
| .map((row) => row.name) | ||
| .filter((name): name is string => typeof name === "string"), | ||
| ); | ||
| }; | ||
|
|
||
| const rebuildLegacyConnectionTable = async (db: D1Database): Promise<void> => { | ||
| const statements = [ | ||
| `DROP TABLE IF EXISTS connection_next`, | ||
| `DROP TABLE IF EXISTS connection_legacy_item_id`, | ||
| `CREATE TABLE connection_next ( | ||
| integration text NOT NULL, | ||
| name text NOT NULL, | ||
| template text NOT NULL, | ||
| provider text NOT NULL, | ||
| item_ids json NOT NULL, | ||
| identity_label text, | ||
| description text, | ||
| tools_synced_at integer, | ||
| oauth_client text, | ||
| oauth_client_owner text, | ||
| refresh_item_id text, | ||
| expires_at integer, | ||
| oauth_scope text, | ||
| oauth_token_url text, | ||
| provider_state text, | ||
| created_at integer NOT NULL, | ||
| updated_at integer NOT NULL, | ||
| row_id text PRIMARY KEY NOT NULL, | ||
| tenant text NOT NULL, | ||
| owner text NOT NULL, | ||
| subject text NOT NULL | ||
| )`, | ||
| `INSERT INTO connection_next | ||
| (integration, name, template, provider, item_ids, identity_label, | ||
| description, tools_synced_at, oauth_client, oauth_client_owner, | ||
| refresh_item_id, expires_at, oauth_scope, oauth_token_url, | ||
| provider_state, created_at, updated_at, row_id, tenant, owner, subject) | ||
| SELECT integration, name, template, provider, | ||
| CASE | ||
| WHEN item_ids IS NOT NULL AND item_ids <> '{}' THEN item_ids | ||
| ELSE json_object('token', item_id) | ||
| END, | ||
| identity_label, description, tools_synced_at, oauth_client, | ||
| oauth_client_owner, refresh_item_id, expires_at, oauth_scope, | ||
| oauth_token_url, provider_state, created_at, updated_at, row_id, | ||
| tenant, owner, subject | ||
| FROM connection`, | ||
| `ALTER TABLE connection RENAME TO connection_legacy_item_id`, | ||
| `ALTER TABLE connection_next RENAME TO connection`, | ||
| `DROP TABLE connection_legacy_item_id`, | ||
| ]; | ||
| for (const statement of statements) { | ||
| await db.prepare(statement).run(); | ||
| } | ||
| }; | ||
|
|
||
| export const ensureCloudflareD1SchemaCompatibility = async (db: D1Database): Promise<void> => { |
There was a problem hiding this comment.
rebuildLegacyConnectionTable executes seven DDL statements with sequential await db.prepare(stmt).run() calls. D1 auto-commits each statement individually, so there is no rollback if a step fails. The critical window is between step 5 (ALTER TABLE connection RENAME TO connection_legacy_item_id) and step 6 (ALTER TABLE connection_next RENAME TO connection): if step 6 fails, the connection table no longer exists, breaking every subsequent startup. Worse, ensureCloudflareD1SchemaCompatibility's early-return guard (if (connectionColumns.size === 0) return) will silently skip the fix on the next boot because PRAGMA table_info('connection') returns zero rows for a non-existent table.
D1 db.batch() provides all-or-nothing semantics for multiple statements. Using it here would make the entire rebuild atomic:
await db.batch(statements.map((sql) => db.prepare(sql)));
RhysSullivan
force-pushed
the
codex/toolkits-scoped-mcp
branch
3 times, most recently
from
bed2d28 to
be7fa25
Compare
RhysSullivan
force-pushed
the
codex/toolkits-scoped-mcp
branch
from
be7fa25 to
2459f6c
Compare
1 participant
Summary
Visual Evidence
First step of the self-host toolkit UI scenario, showing toolkit cards with the refined layout and no path/scope/date metadata:
Validation
bun run lintbun run --cwd packages/plugins/toolkits typecheckbun run --cwd e2e test:selfhost selfhost/toolkits-ui.test.tsbunx oxfmt --check apps/host-cloudflare/src/db/data-migrations.ts apps/host-cloudflare/src/db/data-migrations.test.ts apps/host-cloudflare/src/mcp/auth.ts apps/host-cloudflare/wrangler.jsonc e2e/scenarios/toolkits-mcp.test.ts e2e/src/surfaces/mcp.ts e2e/setup/cloudflare.boot.ts e2e/local/toolkits-mcp.test.tsbun run --cwd apps/local typecheck(passes with existing typed-error warnings inapps/local/src/main.ts)bun run --cwd apps/host-cloudflare typecheckbun run --cwd apps/host-cloudflare test -- src/db/data-migrations.test.tsbun run --cwd packages/plugins/google test -- src/sdk/openapi-ownership-migration.test.tscd e2e && bunx --bun vitest run --project local local/toolkits-mcp.test.tscd e2e && E2E_NODE_BIN=/opt/homebrew/bin/node bun run test:cloudflare scenarios/toolkits-mcp.test.tscd e2e && bun run test:selfhost scenarios/toolkits-mcp.test.tscd e2e && bun run test:cloud scenarios/toolkits-mcp.test.tsNotes
cd e2e && bun run typecheckis currently blocked by the unrelated untrackede2e/scripts/google-oauth-matrix.ts, which has missing imports and type errors outside this PR scope.Header Polish Evidence
Toolkit detail header after removing the duplicate tool count and tightening the top bar:
Additional validation for the header polish:
bunx oxfmt --check packages/plugins/toolkits/src/page.tsxgit diff --check -- packages/plugins/toolkits/src/page.tsxbun run typecheckinpackages/plugins/toolkitsbun run testinpackages/plugins/toolkitsbunx vitest run --project selfhost selfhost/toolkits-ui.test.tsine2eHidden Personal Connections Note
The manage-connections dialog now explains when personal connections are hidden from a shared workspace toolkit:
Additional validation for the note:
bunx oxfmt --check packages/plugins/toolkits/src/page.tsx e2e/selfhost/toolkits-ui.test.tsgit diff --check -- packages/plugins/toolkits/src/page.tsx e2e/selfhost/toolkits-ui.test.tsbun run typecheckinpackages/plugins/toolkitsbun run testinpackages/plugins/toolkitsbunx tsc --noEmitine2eE2E_SELFHOST_URL=http://100.81.219.45:44614 E2E_SELFHOST_ADMIN_EMAIL=admin@e2e.test E2E_SELFHOST_ADMIN_PASSWORD=e2e-admin-password-123 bunx vitest run --project selfhost selfhost/toolkits-ui.test.tsine2e