Repository navigation
Add @kody/codemode-utils and rename generated UI utils alias - #88
Closed
kentcdodds wants to merge 8 commits into
Closed
kentcdodds wants to merge 8 commits into
kentcdodds wants to merge 8 commits into
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThe pull request renames the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
kentcdodds
commented
Mar 29, 2026
| }; | ||
| }\` | ||
|
|
||
| \`import { refreshAccessToken } from '@kody/codemode-utils' |
Owner
Author
There was a problem hiding this comment.
@cursoragent this won't work because the code should be in an arrow function. Instead show a dynamic import with await
Contributor
There was a problem hiding this comment.
Summary
- Updated the
executetool example to keep the code inside an async arrow function and useconst { refreshAccessToken } = await import('@kody/codemode-utils'), matching Kent’s review feedback and the intended execute usage.execute.ts:72-87
Testing
- ✅
npx tsc -p tsconfig.json --noEmit
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
kentcdodds
marked this pull request as ready for review
March 29, 2026 03:11
Contributor
|
🔎 Preview deployed: https://kody-pr-88.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.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/worker/src/mcp/executor.ts (1)
76-84: Module detection heuristic may have false positives.The regex
^\s*import\sand^\s*export\swill match comments containing these words (e.g.,// import something). Consider using the AST-based detection fromwrapImportedExecuteCodeearlier, or ensure comments are handled.💡 Consider using acorn parse for initial detection too
function buildUserEntryModuleSource(code: string) { const trimmed = code.trim() - const looksLikeModule = - /^\s*import\s/m.test(trimmed) || /^\s*export\s/m.test(trimmed) - if (looksLikeModule) { - return wrapImportedExecuteCode(trimmed) + try { + const parsed = acorn.parse(trimmed, { + ecmaVersion: 'latest', + sourceType: 'module', + }) + const hasImportsOrExports = parsed.body.some( + (node) => + node.type === 'ImportDeclaration' || + node.type === 'ExportDefaultDeclaration' || + node.type === 'ExportNamedDeclaration' || + node.type === 'ExportAllDeclaration', + ) + if (hasImportsOrExports) { + return wrapImportedExecuteCode(trimmed) + } + } catch { + // Fall back to arrow function wrapping if parse fails } return `export default ${normalizeCode(code)};` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/executor.ts` around lines 76 - 84, The module-detection regex in buildUserEntryModuleSource can false-positive on imports/exports inside comments; replace the simple regex check with the same AST-based detection used by wrapImportedExecuteCode (or run acorn.parse on the trimmed code and test the AST for ImportDeclaration/ExportNamedDeclaration/ExportDefaultDeclaration/ExportAllDeclaration nodes) so comments are ignored, then call wrapImportedExecuteCode when the AST shows module syntax; otherwise return the existing export default using normalizeCode.packages/worker/client/mcp-apps/generated-ui-runtime-contract.ts (1)
46-53: Consider a temporary deprecated alias for backward compatibility.Line 46 switches the runtime alias globally, and Lines 51-53 emit only that key. If existing saved apps still import
@kody/utils, they will fail at runtime after rollout. Consider mapping both aliases during migration.Suggested migration-safe diff
export const generatedUiRuntimeModuleSpecifier = '@kody/ui-utils' as const +export const deprecatedGeneratedUiRuntimeModuleSpecifier = '@kody/utils' as const export function buildGeneratedUiRuntimeImportMap(runtimeScriptHref: string) { const importMapJson = escapeInlineScriptSource( JSON.stringify({ imports: { [generatedUiRuntimeModuleSpecifier]: runtimeScriptHref, + [deprecatedGeneratedUiRuntimeModuleSpecifier]: runtimeScriptHref, }, }), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/client/mcp-apps/generated-ui-runtime-contract.ts` around lines 46 - 53, buildGeneratedUiRuntimeImportMap currently only maps the new alias stored in generatedUiRuntimeModuleSpecifier to runtimeScriptHref which will break saved apps importing the old alias; update the function to also emit a deprecated alias mapping (e.g., '@kody/utils') pointing to the same runtimeScriptHref so both import keys are present during migration — you can add a new constant like deprecatedUiRuntimeModuleSpecifier and include both [generatedUiRuntimeModuleSpecifier] and [deprecatedUiRuntimeModuleSpecifier] in the imports object returned by buildGeneratedUiRuntimeImportMap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/executor.ts`:
- Line 216: Update the compatibilityDate value to the current runtime date:
locate the compatibilityDate key (currently '2025-06-01') in the configuration
object in executor.ts and change its string value to the current date (e.g.,
'2026-03-29') so the Workers runtime uses up-to-date feature availability.
- Around line 214-231: The TypeScript error occurs because entrypoint (from
input.loader.get(...).getEntrypoint()) is typed as Fetcher<undefined> and lacks
the custom evaluate method; fix by asserting a proper type that exposes evaluate
(e.g., the dynamic module's executor interface or Fetcher<CodeExecutor>) so you
can call entrypoint.evaluate(dispatchers) and get an ExecuteResult; update the
getEntrypoint() call or the entrypoint variable declaration to include the
correct generic/type assertion referencing input.loader, getEntrypoint,
entrypoint, evaluate, and ExecuteResult (or the CodeExecutor export) so
TypeScript recognizes the evaluate method.
In `@packages/worker/src/mcp/tools/codemode-utils.ts`:
- Around line 60-63: The connector routing metadata parsed into
requiredHosts/allowedHosts is never enforced in createAuthenticatedFetch,
allowing bearer-authenticated calls to arbitrary hosts; update
createAuthenticatedFetch to validate outbound request URLs against the parsed
routing policy (e.g., enforce a baseUrl prefix and/or match the request host
against value.requiredHosts or allowedHosts) and reject or rewrite requests that
don't comply; locate the routing data created where requiredHosts is filtered
and pass it into (or reference it from) createAuthenticatedFetch (the fetch
wrapper) so every fetch call verifies host/base-URL constraints before sending
the Authorization header.
---
Nitpick comments:
In `@packages/worker/client/mcp-apps/generated-ui-runtime-contract.ts`:
- Around line 46-53: buildGeneratedUiRuntimeImportMap currently only maps the
new alias stored in generatedUiRuntimeModuleSpecifier to runtimeScriptHref which
will break saved apps importing the old alias; update the function to also emit
a deprecated alias mapping (e.g., '@kody/utils') pointing to the same
runtimeScriptHref so both import keys are present during migration — you can add
a new constant like deprecatedUiRuntimeModuleSpecifier and include both
[generatedUiRuntimeModuleSpecifier] and [deprecatedUiRuntimeModuleSpecifier] in
the imports object returned by buildGeneratedUiRuntimeImportMap.
In `@packages/worker/src/mcp/executor.ts`:
- Around line 76-84: The module-detection regex in buildUserEntryModuleSource
can false-positive on imports/exports inside comments; replace the simple regex
check with the same AST-based detection used by wrapImportedExecuteCode (or run
acorn.parse on the trimmed code and test the AST for
ImportDeclaration/ExportNamedDeclaration/ExportDefaultDeclaration/ExportAllDeclaration
nodes) so comments are ignored, then call wrapImportedExecuteCode when the AST
shows module syntax; otherwise return the existing export default using
normalizeCode.
🪄 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
Run ID: 0f4f5037-7504-41b0-b196-26726044bd18
📒 Files selected for processing (16)
packages/worker/client/mcp-apps/generated-ui-runtime-contract.tspackages/worker/client/mcp-apps/generated-ui-widget-runtime.tspackages/worker/src/app/saved-ui-hosted-html.node.test.tspackages/worker/src/mcp/capabilities/apps/ui-save-app.tspackages/worker/src/mcp/capabilities/apps/ui-update-app.tspackages/worker/src/mcp/capabilities/coding/generated-ui-oauth-guide.node.test.tspackages/worker/src/mcp/capabilities/coding/generated-ui-oauth-guide.tspackages/worker/src/mcp/executor.tspackages/worker/src/mcp/generated-ui-api.tspackages/worker/src/mcp/generated-ui-app-base-url.node.test.tspackages/worker/src/mcp/index.tspackages/worker/src/mcp/mcp-server.mcp-e2e.test.tspackages/worker/src/mcp/run-codemode-registry.workers.test.tspackages/worker/src/mcp/tools/codemode-utils.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/mcp/tools/open-generated-ui.ts
|
|
||
| const entrypoint = input.loader | ||
| .get(`codemode-${crypto.randomUUID()}`, () => ({ | ||
| compatibilityDate: '2025-06-01', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are other compatibility dates used in the codebase for reference
rg -n "compatibilityDate" --type ts -C1Repository: kentcdodds/kody
Length of output: 552
🏁 Script executed:
#!/bin/bash
# Get full context around line 216 in executor.ts
sed -n '200,230p' packages/worker/src/mcp/executor.tsRepository: kentcdodds/kody
Length of output: 944
🏁 Script executed:
#!/bin/bash
# Search for any comments or documentation about compatibility dates
rg -i "compatibility" packages/worker/src/ -A 2 -B 2 --type ts | head -50Repository: kentcdodds/kody
Length of output: 1329
🏁 Script executed:
#!/bin/bash
# Look for any config files that might set or reference compatibility dates
fd -e json -e toml -e yaml -e yml packages/worker | xargs grep -l "2025" 2>/dev/null || echo "No matches"Repository: kentcdodds/kody
Length of output: 452
🌐 Web query:
Cloudflare Workers compatibility date format valid dates 2025-06-01
💡 Result:
The compatibility date for Cloudflare Workers uses the YYYY-MM-DD date format (e.g., "2022-04-05"). The date "2025-06-01" follows this format exactly and is a valid past date as of today (2026-03-29). Official documentation consistently shows examples in this format, and Wrangler validates dates, rejecting invalid ones (e.g., future dates like "2024-11-05" in 2024 caused issues). Compatibility dates must be in the past or present; the runtime supports all historical dates from 2021-11-02 onward indefinitely. Flags enable at specific dates up to 2026-04-01, confirming support for 2025 dates. Use the latest date like "2026-03-29" for new projects to opt into all changes.
Citations:
- 1: https://developers.cloudflare.com/workers/configuration/compatibility-dates/
- 2: https://developers.cloudflare.com/workers/configuration/compatibility-flags/
- 3: https://developers.cloudflare.com/workers/platform/compatibility-dates
- 4: https://developers.cloudflare.com/workers/platform/changelog/
- 5: https://developers.cloudflare.com/workers/configuration/compatibility-flags
- 6: https://cfdata.lol/workerd/
Update compatibility date to current version.
The compatibility date 2025-06-01 is outdated. Since we are in March 2026, this date is approximately 9 months old. For optimal compatibility and access to recent runtime features, update this to the current date (e.g., 2026-03-29). Cloudflare Workers supports all historical dates from 2021-11-02 onward, so there is no risk in updating to a newer date.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/executor.ts` at line 216, Update the
compatibilityDate value to the current runtime date: locate the
compatibilityDate key (currently '2025-06-01') in the configuration object in
executor.ts and change its string value to the current date (e.g., '2026-03-29')
so the Workers runtime uses up-to-date feature availability.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/worker/src/mcp/executor.ts (1)
229-246:⚠️ Potential issue | 🔴 CriticalType the loaded entrypoint before calling
evaluate(TS2339 blocker).Line [244] returns a fetcher-typed stub by default, so Line [246] fails typecheck when calling
evaluate.Proposed fix
+type CodeExecutorEntrypoint = { + evaluate(dispatchers: Record<string, ToolDispatcher>): Promise<ExecuteResult> +} + const entrypoint = input.loader .get(`codemode-${crypto.randomUUID()}`, () => ({ compatibilityDate: '2025-06-01', compatibilityFlags: ['nodejs_compat'], mainModule: 'executor.js', modules: { [`${codemodeUtilsModuleSpecifier}`]: { js: buildCodemodeUtilsModuleSource(), }, 'user-entry.js': buildUserEntryModuleSource(input.code), 'user-code.js': buildUserCodeRunnerModuleSource(), 'executor.js': executorModuleSource, }, globalOutbound: input.globalOutbound, })) - .getEntrypoint() + .getEntrypoint() as unknown as CodeExecutorEntrypoint const response = (await entrypoint.evaluate(dispatchers)) as ExecuteResult#!/bin/bash set -euo pipefail echo "1) Show the call site that triggers TS2339:" sed -n '229,248p' packages/worker/src/mcp/executor.ts echo echo "2) Inspect WorkerLoader/entrypoint typing in generated worker types:" fd -i "worker-configuration.d.ts" packages/worker --exec rg -n -C2 'getEntrypoint' echo echo "3) Locate any existing TS2339 signal references:" rg -n -C2 "TS2339|Property 'evaluate' does not exist"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/executor.ts` around lines 229 - 246, The entrypoint returned by input.loader.get(...).getEntrypoint() is currently typed as the fetcher stub so calling entrypoint.evaluate causes TS2339; update the typing at the call site by casting or narrowing the result to the proper Worker Entrypoint type before calling evaluate (e.g. assert the type that exposes evaluate on the object returned by getEntrypoint), or change the loader/getEntrypoint typing to return the concrete entrypoint type; modify the expression assigning entrypoint (the chain starting at input.loader.get(...).getEntrypoint()) so entrypoint is typed to include evaluate and then call entrypoint.evaluate(dispatchers) as ExecuteResult.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/executor.ts`:
- Around line 159-181: The provider-name validation must also block
prototype/property collision names and avoid prototype-polluting plain objects:
add "__proto__", "prototype", and "constructor" to the reservedNames set (in
addition to '__dispatchers','__logs','__providers') and keep the syntactic check
via validIdentifier and duplicate check via seenNames; also replace uses of
plain object literals used as maps for providers/logs with objects created via
Object.create(null) (i.e., create maps with null-prototype) where dynamic writes
occur so assigning provider names cannot mutate prototypes.
---
Duplicate comments:
In `@packages/worker/src/mcp/executor.ts`:
- Around line 229-246: The entrypoint returned by
input.loader.get(...).getEntrypoint() is currently typed as the fetcher stub so
calling entrypoint.evaluate causes TS2339; update the typing at the call site by
casting or narrowing the result to the proper Worker Entrypoint type before
calling evaluate (e.g. assert the type that exposes evaluate on the object
returned by getEntrypoint), or change the loader/getEntrypoint typing to return
the concrete entrypoint type; modify the expression assigning entrypoint (the
chain starting at input.loader.get(...).getEntrypoint()) so entrypoint is typed
to include evaluate and then call entrypoint.evaluate(dispatchers) as
ExecuteResult.
🪄 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
Run ID: b36fac44-186d-4e30-b0c5-ef0c848e3951
📒 Files selected for processing (2)
packages/worker/src/mcp/executor.tspackages/worker/src/mcp/tools/codemode-utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/src/mcp/tools/codemode-utils.ts
Comment on lines
+159
to
+181
| const reservedNames = new Set(['__dispatchers', '__logs', '__providers']) | ||
| const validIdentifier = /^[a-zA-Z_$][a-zA-Z0-9_$]*$/ | ||
| const seenNames = new Set<string>() | ||
| for (const provider of input.providers) { | ||
| if (reservedNames.has(provider.name)) { | ||
| return { | ||
| result: undefined, | ||
| error: `Provider name "${provider.name}" is reserved`, | ||
| } | ||
| } | ||
| if (!validIdentifier.test(provider.name)) { | ||
| return { | ||
| result: undefined, | ||
| error: `Provider name "${provider.name}" is not a valid JavaScript identifier`, | ||
| } | ||
| } | ||
| if (seenNames.has(provider.name)) { | ||
| return { | ||
| result: undefined, | ||
| error: `Duplicate provider name "${provider.name}"`, | ||
| } | ||
| } | ||
| seenNames.add(provider.name) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Current provider-name validation and dynamic map writes:"
sed -n '159,227p' packages/worker/src/mcp/executor.ts
echo
echo "2) Trace provider name usage/definition points:"
rg -n -C3 --type=ts '\bResolvedProvider\b|provider\.name|name:\s*string'Repository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
# Verify the exact locations and check for other plain object usages
grep -n "Object.create\|const.*=.*{}" packages/worker/src/mcp/executor.ts | head -20Repository: kentcdodds/kody
Length of output: 273
Harden provider-name handling against prototype-key collisions.
Provider names like __proto__, constructor, and prototype pass validation at lines 159-181 because the validIdentifier regex only checks for syntactic validity, not reserved property names. Subsequent dynamic writes at lines 194 and 215 using plain {} objects can mutate prototypes unexpectedly.
Proposed fix
- const reservedNames = new Set(['__dispatchers', '__logs', '__providers'])
+ const reservedNames = new Set([
+ '__dispatchers',
+ '__logs',
+ '__providers',
+ '__proto__',
+ 'prototype',
+ 'constructor',
+ ])
@@
- ' const __providers = {};',
+ ' const __providers = Object.create(null);',
@@
- const dispatchers = {} as Record<string, ToolDispatcher>
+ const dispatchers: Record<string, ToolDispatcher> = Object.create(null)Also applies to: 194-199, 215-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/executor.ts` around lines 159 - 181, The
provider-name validation must also block prototype/property collision names and
avoid prototype-polluting plain objects: add "__proto__", "prototype", and
"constructor" to the reservedNames set (in addition to
'__dispatchers','__logs','__providers') and keep the syntactic check via
validIdentifier and duplicate check via seenNames; also replace uses of plain
object literals used as maps for providers/logs with objects created via
Object.create(null) (i.e., create maps with null-prototype) where dynamic writes
occur so assigning provider names cannot mutate prototypes.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
- Root cause: always attaching the virtual @kody/codemode-utils module to the WorkerLoader graph broke RPC (DataCloneError / MCP serialization), so structuredContent was missing and app_id looked undefined in E2E. - Use @cloudflare/codemode DynamicWorkerExecutor for normal snippets; only merge the codemode-utils module when the code references it. - When utils are referenced, run a small fork of the codemode executor template that sets globalThis.__kodyProviders so requireCodemode() works. - Reject top-level import/export with a clear error (multi-module loader + RPC is not supported); document dynamic import() in the execute tool. - JSON-round-trip structuredContent.result in the execute tool for MCP safety. - Fix node mock for export default class; remove flaky workers integration test. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/worker/src/mcp/executor.ts (1)
83-111:⚠️ Potential issue | 🟡 MinorAdd prototype-polluting property names to reserved set.
Provider names like
__proto__,constructor, andprototypepass validation because thevalidIdentifierregex only checks syntactic validity. These should be blocked to prevent prototype pollution when building dynamic objects.🛡️ Proposed fix
function validateExecuteProviders( providers: Array<ResolvedProvider>, ): ExecuteResult | null { - const reservedNames = new Set(['__dispatchers', '__logs']) + const reservedNames = new Set([ + '__dispatchers', + '__logs', + '__proto__', + 'prototype', + 'constructor', + ]) const validIdentifier = /^[a-zA-Z_$][a-zA-Z0-9_$]*$/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/executor.ts` around lines 83 - 111, The validateExecuteProviders function currently only reserves '__dispatchers' and '__logs' but allows prototype-polluting names; update the reservedNames set in validateExecuteProviders to include '__proto__', 'constructor', 'prototype' (and any other Object prototype keys you deem necessary) so names that would mutate object prototypes are rejected, keeping the existing error-return behavior (results with error messages) when provider.name matches one of these reserved keys.
🧹 Nitpick comments (2)
packages/worker/src/mcp/executor.node.test.ts (1)
152-163: Fix ESLintconsistent-type-importsviolation.The
typeof import('cloudflare:workers').exportssyntax is flagged by ESLint. Use a type-only import or type alias instead.♻️ Proposed fix
+import type { exports as WorkerExportsType } from 'cloudflare:workers' + // ... in test ... exports: { CodemodeFetchGateway() { return {} as Fetcher }, - } as typeof import('cloudflare:workers').exports, + } as typeof WorkerExportsType,Alternatively, if module augmentation is complex, you can use a simple inline type:
exports: { CodemodeFetchGateway() { return {} as Fetcher }, - } as typeof import('cloudflare:workers').exports, + } as { CodemodeFetchGateway: () => Fetcher },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/executor.node.test.ts` around lines 152 - 163, Replace the inline value assertion using typeof import(...) for the exports property with a type-only import or alias to satisfy ESLint; add a top-level type import (e.g., import type { exports as CloudflareExports } from 'cloudflare:workers') and then cast the mock exports object in this test (the exports object returned for CodemodeFetchGateway / Fetcher) to that CloudflareExports type instead of using typeof import('cloudflare:workers').exports; update the test file's exports: { CodemodeFetchGateway() { return {} as Fetcher } } assertion to use the new imported type.packages/worker/src/mcp/executor.ts (1)
113-141: Generated code creates null-prototype objects where applicable.The proxy-based provider bindings are correctly scoped. However, line 116 generates
const __kodyProviders = {};which creates a regular object. While the risk is lower in the sandboxed worker context since__kodyProvidersis not used as a lookup map with untrusted keys, usingObject.create(null)would be more defensive.♻️ Proposed fix for defense-in-depth
function buildKodyUtilsProviderSetupLines( providers: Array<ResolvedProvider>, ): Array<string> { - const lines: Array<string> = [' const __kodyProviders = {};'] + const lines: Array<string> = [' const __kodyProviders = Object.create(null);']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/executor.ts` around lines 113 - 141, The created __kodyProviders object uses a normal object literal which has a prototype; change the initialization in buildKodyUtilsProviderSetupLines so __kodyProviders is created with no prototype (use Object.create(null)) to avoid inherited keys, keep the rest of the function identical (the Proxy constructions for each provider p.name, the assignments to __kodyProviders[p.name], and the final globalThis.__kodyProviders export should remain unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/executor.ts`:
- Around line 72-78: The type mismatch comes from passing mergedModules
(Record<string, WorkerLoaderModule | string> | undefined) into the
DynamicWorkerExecutor which expects Record<string, string> | undefined; in the
branch where codeReferencesCodemodeUtils(code) is false modules is never
populated so pass undefined directly to DynamicWorkerExecutor (or remove the
mergedModules intermediate and pass undefined into the constructor) so that the
constructor's modules parameter matches the expected type; update the code that
constructs new DynamicWorkerExecutor({ loader: input.env.LOADER, timeout:
90_000, globalOutbound, modules: ... }) to use modules: undefined when
codeReferencesCodemodeUtils(code) is false and keep execute(code,
providersOrFns) unchanged.
---
Duplicate comments:
In `@packages/worker/src/mcp/executor.ts`:
- Around line 83-111: The validateExecuteProviders function currently only
reserves '__dispatchers' and '__logs' but allows prototype-polluting names;
update the reservedNames set in validateExecuteProviders to include '__proto__',
'constructor', 'prototype' (and any other Object prototype keys you deem
necessary) so names that would mutate object prototypes are rejected, keeping
the existing error-return behavior (results with error messages) when
provider.name matches one of these reserved keys.
---
Nitpick comments:
In `@packages/worker/src/mcp/executor.node.test.ts`:
- Around line 152-163: Replace the inline value assertion using typeof
import(...) for the exports property with a type-only import or alias to satisfy
ESLint; add a top-level type import (e.g., import type { exports as
CloudflareExports } from 'cloudflare:workers') and then cast the mock exports
object in this test (the exports object returned for CodemodeFetchGateway /
Fetcher) to that CloudflareExports type instead of using typeof
import('cloudflare:workers').exports; update the test file's exports: {
CodemodeFetchGateway() { return {} as Fetcher } } assertion to use the new
imported type.
In `@packages/worker/src/mcp/executor.ts`:
- Around line 113-141: The created __kodyProviders object uses a normal object
literal which has a prototype; change the initialization in
buildKodyUtilsProviderSetupLines so __kodyProviders is created with no prototype
(use Object.create(null)) to avoid inherited keys, keep the rest of the function
identical (the Proxy constructions for each provider p.name, the assignments to
__kodyProviders[p.name], and the final globalThis.__kodyProviders export should
remain unchanged).
🪄 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
Run ID: 504f16b0-d9a9-47b2-b254-4912b7507174
📒 Files selected for processing (3)
packages/worker/src/mcp/executor.node.test.tspackages/worker/src/mcp/executor.tspackages/worker/src/mcp/tools/execute.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/src/mcp/tools/execute.ts
| .string() | ||
| .describe('JavaScript async arrow function to execute capabilities.'), | ||
| .describe( | ||
| 'JavaScript execute code. Use an async arrow function, or top-level imports with normal statements/returns.', |
Contributor
There was a problem hiding this comment.
Schema description contradicts actual top-level import rejection
High Severity
The inputSchema.code.describe text says "Use an async arrow function, or top-level imports with normal statements/returns," but codeNeedsModuleRunner in executor.ts explicitly rejects top-level imports with an error message. The longer tool description (lines 65–67) correctly says "Do not use top-level import or export." This contradiction in the schema description will cause LLMs to generate code with top-level imports that the executor immediately rejects.
Additional Locations (1)
|
|
||
| export async function createAuthenticatedFetch(providerName) { | ||
| const connector = await readConnectorConfig(providerName) | ||
| const accessToken = await refreshAccessToken(providerName) |
Contributor
There was a problem hiding this comment.
Double connector config fetch in createAuthenticatedFetch
Low Severity
createAuthenticatedFetch calls readConnectorConfig(providerName) on line 256, then calls refreshAccessToken(providerName) on line 257, which internally calls readConnectorConfig(providerName) again on line 207. This results in two redundant async API calls (connector_get or value_get) for the same provider name when only one is needed.
| if (parsed) { | ||
| return parsed | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Proxy defeats typeof feature detection in codemode-utils
Medium Severity
The typeof codemode.connector_get === 'function' guard in readConnectorConfig is meant to gracefully fall back to value_get when connector_get isn't available. However, codemode is a Proxy (from buildKodyUtilsProviderSetupLines) whose get trap returns an async function for every property access. This makes the typeof check always true, so the call always goes through. If the dispatcher lacks connector_get, it throws a hard error instead of falling through to the value_get fallback path.
Additional Locations (1)
| globalOutbound, | ||
| modules: mergedDynamicExecutorModules, | ||
| }) | ||
| return executor.execute(code, providersOrFns) |
Contributor
There was a problem hiding this comment.
Dead code: dynamicExecutorModules branch is never reached
Low Severity
dynamicExecutorModules and mergedDynamicExecutorModules are populated only when codeReferencesCodemodeUtils(code) is true (line 57), but that same condition (line 72) routes to the bridge path instead. The standard DynamicWorkerExecutor path (line 82–88) only runs when the condition is false, meaning mergedDynamicExecutorModules is always undefined when it's used.
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
@kody/utilsto@kody/ui-utils@kody/codemode-utilswithrefreshAccessToken()andcreateAuthenticatedFetch()DynamicWorkerExecutor, while using a focused bridge only for@kody/codemode-utilsTesting
execute+ui_save_appCloses #83.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes