feat: run FrontMCP on Cloudflare Workers (web-native fetch handler + V8-isolate-clean SDK) + vectoriadb peer hardening#492
Merged
Conversation
added 2 commits
June 15, 2026 14:32
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Cloudflare Workers support with a Web fetch transport, worker-safe lazy initialization, managed bundle refresh plumbing, Cloudflare KV storage, and updated Cloudflare deployment configs, docs, and E2E coverage. ChangesCloudflare edge runtime support and worker-safe initialization
Sequence Diagram(s)sequenceDiagram
participant createEdgeMcp
participant SkilledOpenApiPlugin
participant SaasPullSource
participant KV bundle cache
participant BundleStore
createEdgeMcp->>SkilledOpenApiPlugin: resolve SKILLED_OPENAPI_RUNTIME_DEPS_TOKEN
SkilledOpenApiPlugin->>SaasPullSource: createBundleSource(..., deps)
SkilledOpenApiPlugin->>SaasPullSource: attach(source)
createEdgeMcp->>SaasPullSource: refresh() on scheduled
SaasPullSource->>KV bundle cache: read/write bundle
SaasPullSource->>BundleStore: notify refreshed bundle
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
frontegg-david
changed the title
vectoriadb in MemorySkillProvider![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/frontmcp/deployment/serverless.mdx (1)
274-281:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCloudflare output paths are now inconsistent within the same page.
The updated
wrangler.tomlcorrectly usesmain = "dist/cloudflare/index.js", but the earlier “This generates” Cloudflare tree still showsdist/index.js. Please align that tree to avoid contradictory deployment instructions.As per coding guidelines, “When public APIs in libs/** or plugins/** change, ensure the matching docs page under docs/frontmcp/** is updated in the same PR.”
Also applies to: 292-296
🤖 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 `@docs/frontmcp/deployment/serverless.mdx` around lines 274 - 281, The Cloudflare output path tree shown in the "This generates" section displays `dist/index.js` as the compiled handler location, but the wrangler.toml configuration correctly specifies `main = "dist/cloudflare/index.js"`. Update the file tree output to show `dist/cloudflare/index.js` instead of `dist/index.js` to ensure consistency with the actual wrangler.toml configuration. This inconsistency also appears at lines 292-296, so apply the same correction there as well to provide accurate and non-contradictory deployment instructions.Source: Coding guidelines
🧹 Nitpick comments (1)
libs/cli/frontmcp.schema.json (1)
314-319: ⚡ Quick winTighten
wranglerschema validation to match runtime contract.
compatibilityDatecurrently allows any string, andcompatibilityFlagsallows empty/unbounded values. Adding basic constraints here catches invalid config before build/deploy time and aligns better with the stricter runtime schema patterns already used elsewhere.Suggested diff
- "compatibilityDate": { "type": "string", "description": "Compatibility date. Defaults to 2024-09-23 (enables full nodejs_compat)." }, + "compatibilityDate": { + "type": "string", + "pattern": "^\\d{4}-\\d{2}-\\d{2}$", + "description": "Compatibility date. Defaults to 2024-09-23 (enables full nodejs_compat)." + }, "compatibilityFlags": { "type": "array", - "items": { "type": "string" }, + "items": { "type": "string", "minLength": 1, "maxLength": 128 }, + "maxItems": 32, + "uniqueItems": true, "description": "Extra Cloudflare compatibility flags. nodejs_compat is always emitted; list only additions." }🤖 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 `@libs/cli/frontmcp.schema.json` around lines 314 - 319, The compatibilityDate and compatibilityFlags properties in the schema need stricter validation constraints to catch invalid configurations earlier. For compatibilityDate, add a pattern constraint to enforce valid date format (e.g., YYYY-MM-DD format like the default example 2024-09-23). For compatibilityFlags array, add minItems and/or maxItems constraints to prevent empty arrays or unbounded lists, and ensure items validation is appropriately restrictive. These changes align the schema with stricter validation patterns used elsewhere and prevent invalid configurations from reaching the build/deploy phase.
🤖 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 `@apps/e2e/demo-e2e-cloudflare/e2e/cloudflare-worker.e2e.spec.ts`:
- Around line 45-53: The mcp function uses `any` in its return type for the json
property, which violates strict TypeScript typing rules. Replace the `any` type
in the Promise return type of the mcp function with either a generic type
parameter (making mcp a generic function that accepts a type parameter for the
json response) or use `unknown`, then explicitly type the response at each call
site where mcp is invoked to ensure strict typing is maintained throughout.
In `@apps/e2e/demo-e2e-cloudflare/project.json`:
- Line 18: The passWithNoTests configuration in the project.json file is
currently set to true, which allows the test suite to pass when no tests are
discovered. This creates a false-green regression check that defeats the purpose
of the E2E safety net. Change the passWithNoTests value from true to false to
ensure the project fails when no tests are found, preventing glob path drift and
other test discovery issues from going undetected. This change applies to all
occurrences of passWithNoTests in the project.json configuration.
In `@libs/sdk/src/front-mcp/front-mcp.ts`:
- Around line 292-297: The memoization of the build() promise in the building
variable causes a permanent failure if the initial build() call rejects. When
build() fails, building stores a rejected promise, and subsequent requests will
fail immediately since the ??= operator won't retry. Wrap the await building
statement in a try-catch block, and in the catch handler, reset building back to
null or undefined before re-throwing the error. This allows the next request to
attempt building again instead of being forever poisoned by the first failure.
In `@libs/sdk/src/index.ts`:
- Around line 418-420: Add reference documentation for the newly exported public
APIs in the docs/frontmcp/sdk-reference/ directory. Create documentation entries
that cover: how to use the createWebFetchHandler() function in Cloudflare
Workers, Deno, and Bun environments; the complete type signatures for
WebFetchHandler and CreateWebFetchHandlerOptions; and a clear explanation of how
these APIs differ from the existing Express/Node.js transport handler. Ensure
the documentation is discoverable and follows the existing documentation
structure and conventions used in the sdk-reference folder.
In `@libs/sdk/src/scope/__tests__/optional-dependency.util.spec.ts`:
- Around line 99-108: The rejection-path tests omit explicit Error class
validation as required by repository test rules. At
libs/sdk/src/scope/__tests__/optional-dependency.util.spec.ts lines 99-108, add
an expect(error).toBeInstanceOf(Error) assertion before the cause check in the
importOptionalPeer test. At
libs/sdk/src/skill/__tests__/memory-skill.provider.optional-peer.spec.ts lines
80-93, capture each rejection object from the promise rejection and add an
assertion to validate it is an instance of Error in addition to the existing
message matching assertions. Both locations need explicit instanceof Error
checks for each error object being tested.
In `@libs/sdk/src/transport/web-fetch-handler.ts`:
- Around line 93-100: The connect() call on line 93 executes outside the
try/finally block, so if it fails, the cleanup in the finally block won't run.
Additionally, the close() call on line 99 is fire-and-forgotten with void, which
can accumulate pending teardowns under load. Move the
mcpServer.connect(transport) call inside the try block so it's covered by the
finally block's cleanup logic, and change the finally block to properly await
the mcpServer.close() call instead of using void to fire-and-forget it, ensuring
teardown completes before the function returns.
---
Outside diff comments:
In `@docs/frontmcp/deployment/serverless.mdx`:
- Around line 274-281: The Cloudflare output path tree shown in the "This
generates" section displays `dist/index.js` as the compiled handler location,
but the wrangler.toml configuration correctly specifies `main =
"dist/cloudflare/index.js"`. Update the file tree output to show
`dist/cloudflare/index.js` instead of `dist/index.js` to ensure consistency with
the actual wrangler.toml configuration. This inconsistency also appears at lines
292-296, so apply the same correction there as well to provide accurate and
non-contradictory deployment instructions.
---
Nitpick comments:
In `@libs/cli/frontmcp.schema.json`:
- Around line 314-319: The compatibilityDate and compatibilityFlags properties
in the schema need stricter validation constraints to catch invalid
configurations earlier. For compatibilityDate, add a pattern constraint to
enforce valid date format (e.g., YYYY-MM-DD format like the default example
2024-09-23). For compatibilityFlags array, add minItems and/or maxItems
constraints to prevent empty arrays or unbounded lists, and ensure items
validation is appropriately restrictive. These changes align the schema with
stricter validation patterns used elsewhere and prevent invalid configurations
from reaching the build/deploy phase.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc6704d1-05a6-452b-a1a5-5aa9ec7d29f3
⛔ Files ignored due to path filters (4)
libs/cli/src/commands/build/__tests__/adapters.spec.tsis excluded by!**/build/**libs/cli/src/commands/build/adapters/cloudflare.tsis excluded by!**/build/**libs/cli/src/commands/build/index.tsis excluded by!**/build/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (41)
.gitignoreapps/e2e/demo-e2e-cloudflare/e2e/cloudflare-worker.e2e.spec.tsapps/e2e/demo-e2e-cloudflare/fixture/frontmcp.config.jsapps/e2e/demo-e2e-cloudflare/fixture/package.jsonapps/e2e/demo-e2e-cloudflare/fixture/src/main.tsapps/e2e/demo-e2e-cloudflare/fixture/wrangler.tomlapps/e2e/demo-e2e-cloudflare/jest.e2e.config.tsapps/e2e/demo-e2e-cloudflare/project.jsonapps/e2e/demo-e2e-cloudflare/tsconfig.e2e.jsonapps/e2e/demo-e2e-cloudflare/tsconfig.jsonapps/e2e/demo-e2e-frontmcp-config/e2e/build-targets.e2e.spec.tsdocs/frontmcp/deployment/serverless.mdxlibs/auth/src/session/__tests__/session-rate-limiter.spec.tslibs/auth/src/session/session-rate-limiter.tslibs/cli/frontmcp.schema.jsonlibs/cli/src/config/frontmcp-config.schema.tslibs/cli/src/config/frontmcp-config.types.tslibs/nx-plugin/src/generators/server/files/cloudflare/wrangler.toml__tmpl__libs/nx-plugin/src/generators/server/server.spec.tslibs/sdk/src/auth/instances/instance.local-primary-auth.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/front-mcp/front-mcp.tslibs/sdk/src/index.tslibs/sdk/src/scope/__tests__/optional-dependency.util.spec.tslibs/sdk/src/scope/optional-dependency.util.tslibs/sdk/src/skill/__tests__/memory-skill.provider.optional-peer.spec.tslibs/sdk/src/skill/providers/memory-skill.provider.tslibs/sdk/src/transport/__tests__/web-fetch-handler.spec.tslibs/sdk/src/transport/build-scoped-server-options.tslibs/sdk/src/transport/in-memory-server.tslibs/sdk/src/transport/index.tslibs/sdk/src/transport/web-fetch-handler.tslibs/skills/catalog/frontmcp-deployment/examples/deploy-to-cloudflare/basic-worker-deploy.mdlibs/skills/catalog/frontmcp-deployment/examples/deploy-to-cloudflare/worker-custom-domain.mdlibs/skills/catalog/frontmcp-deployment/examples/deploy-to-cloudflare/worker-with-kv-storage.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.mdlibs/skills/catalog/frontmcp-production-readiness/examples/production-cloudflare/durable-objects-state.mdlibs/skills/catalog/frontmcp-production-readiness/examples/production-cloudflare/wrangler-config.mdlibs/utils/src/machine-id/machine-id.tspackage.jsonscripts/inject-esm-require-banner.js
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-06-21T15:45:55.954Z |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@libs/edge/jest.config.ts`:
- Around line 41-47: The coverageThreshold object in the jest.config.ts file has
coverage gates that fall below the library baseline standard of 95%+. Update all
four metrics (statements, branches, functions, and lines) within the global
coverageThreshold configuration to 95 to align with the required library
coverage guidelines and strengthen CI protection for this publishable package.
In `@libs/edge/project.json`:
- Line 61: The outputs configuration in the project.json file at line 61
references an incorrect coverage directory path. Update the outputs array value
from {workspaceRoot}/coverage/unit/worker to {workspaceRoot}/coverage/unit/edge
to align with where the Jest configuration for this edge project actually writes
its coverage reports. This ensures Nx properly tracks the actual output location
generated by the test configuration.
In `@libs/edge/src/index.ts`:
- Around line 109-112: The fetch method caches the handlerPromise using the
nullish coalescing operator, but if build() rejects, that rejected promise
remains cached and all subsequent requests fail permanently. Add error handling
to clear the handlerPromise cache when the promise rejects, either by wrapping
the build() call in a try-catch and resetting handlerPromise to undefined on
error, or by attaching a catch handler to the handlerPromise that clears the
cache before re-throwing the error. This ensures that a subsequent request can
attempt to rebuild the handler instead of reusing the permanently rejected
promise.
In `@libs/sdk/src/common/decorators/__tests__/functional-factories.spec.ts`:
- Around line 69-99: Add negative test cases to validate that invalid app()
metadata throws the expected decorator metadata error type. Create new test
cases (using it() blocks) that pass malformed or invalid metadata to the app()
function or its decorators (e.g., missing required fields, invalid protocol
versions, or malformed parameters), then assert that the thrown error is an
instance of the appropriate metadata error class using instanceof checks. This
ensures error conditions are validated and the correct error types are being
thrown as per the constructor validation guidelines.
- Around line 66-67: In the result async function that handles the JSON-RPC
request and returns the parsed JSON response, replace the return type annotation
from Promise<any> to Promise<unknown>. This maintains type safety while still
allowing the response to be narrowed to a more specific type at assertion sites
where the result is used.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 88d114c0-1aeb-4718-979f-bfa4b082aab3
📒 Files selected for processing (21)
apps/e2e/demo-e2e-cloudflare/e2e/worker-isolate-safety.e2e.spec.tsdocs/frontmcp/deployment/cloudflare-worker.mdxlibs/auth/src/machine-id/__tests__/machine-id.spec.tslibs/auth/src/machine-id/machine-id.tslibs/edge/README.mdlibs/edge/jest.config.tslibs/edge/package.jsonlibs/edge/project.jsonlibs/edge/src/__tests__/managed.spec.tslibs/edge/src/index.spec.tslibs/edge/src/index.tslibs/edge/src/managed.tslibs/edge/tsconfig.jsonlibs/edge/tsconfig.lib.jsonlibs/edge/tsconfig.spec.jsonlibs/sdk/src/common/decorators/__tests__/functional-factories.spec.tslibs/sdk/src/common/decorators/app.decorator.tslibs/sdk/src/index.tslibs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.mdscripts/check-worker-isolate-safety.mjstsconfig.base.json
✅ Files skipped from review due to trivial changes (5)
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.md
- libs/edge/tsconfig.spec.json
- docs/frontmcp/deployment/cloudflare-worker.mdx
- libs/edge/tsconfig.json
- libs/edge/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/index.ts
…dencies in bundle sources
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@apps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.ts`:
- Around line 111-113: The JWKS route matching in the mock server handler is
checking for URLs that start with '/jwks', but the actual injected jwksUrl is
constructed as endpoint plus '/jwks' (which evaluates to '/bundle/jwks' since
endpoint is '/bundle'). Update the conditional check in the if statement that
inspects `req.url?.startsWith('/jwks')` to instead match the full path
'/bundle/jwks' or use a pattern that correctly accounts for the endpoint prefix,
so JWKS requests are properly routed to the JWKS response and not mistaken for
bundle requests.
- Around line 174-186: Remove the unsafe type casts in the request helper
functions. In the fetchWorker function, replace the `as never` cast on the
worker.fetch call with a proper Request object constructor or appropriate type
annotation. In the mcp function, replace the `as any` cast on the res.json()
result with a specific type interface or type definition that properly describes
the expected response structure instead of bypassing type checking.
In `@libs/edge/src/__tests__/kv-cache.spec.ts`:
- Line 82: Remove the unsafe non-null assertion operator from the cache variable
in the test file. Instead of using cache! in the write method call on line 82,
add an explicit guard statement to check if cache exists before using it,
ensuring strict TypeScript correctness. Apply the same fix to any other
instances of non-null assertions (such as the one at line 94) to maintain
consistent coding standards throughout the test file.
In `@libs/sdk/src/esm-loader/esm-cache.ts`:
- Around line 183-190: The evictIfNeeded() method currently only runs after
put() operations, but the get() method loads entries from disk into memoryStore
without triggering eviction, which can cause unbounded memory growth during
cold-read workloads. Add a call to evictIfNeeded() in the get() method after
entries are hydrated from disk into memoryStore (around line 235 where entries
are added from disk) to ensure the maxEntries cap is enforced consistently
across both write and read paths.
In `@libs/sdk/src/logger/instances/instance.file-logger.ts`:
- Around line 6-14: The getFs function in instance.file-logger.ts bypasses the
filesystem abstraction boundary by using direct require('fs') for low-level
synchronous operations like openSync, writeSync, closeSync, and unlinkSync that
are not exposed through `@frontmcp/utils`. Either extend `@frontmcp/utils/fs` to
export these file descriptor operations using the same lazy-load pattern already
established there, or explicitly document in CLAUDE.md that Node-only CLI
features may use direct require('fs') when necessary operations are unavailable
through utils, provided all call sites remain wrapped in try/catch for silent
degradation on non-Node runtimes.
In `@libs/utils/src/storage/adapters/cloudflare-kv.ts`:
- Around line 67-74: The ping() method in the CloudflareKV adapter does not
respect the connection state and can return true even after disconnect() is
called. Add a check at the beginning of the ping() method to verify
this.connected is true before attempting the kv.list() operation. If
this.connected is false, the method should return false immediately to honor the
adapter's connection state contract.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a90cf961-34e7-4cf9-a65c-64292900d005
📒 Files selected for processing (32)
CLAUDE.mdapps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.tsapps/e2e/demo-e2e-cloudflare/fixture-managed/stub-empty.mjsapps/e2e/demo-e2e-cloudflare/fixture-managed/worker.tsapps/e2e/demo-e2e-cloudflare/project.jsonlibs/adapters/src/skills/__tests__/manifest-to-config.spec.tslibs/adapters/src/skills/__tests__/saas-pull.source.edge.spec.tslibs/adapters/src/skills/deploy/manifest-to-config.tslibs/adapters/src/skills/index.tslibs/adapters/src/skills/sources/index.tslibs/adapters/src/skills/sources/npm.source.tslibs/adapters/src/skills/sources/saas-pull.source.tslibs/adapters/src/skills/sources/skill-bundle-source.interface.tslibs/edge/src/__tests__/kv-cache.spec.tslibs/edge/src/index.spec.tslibs/edge/src/index.tslibs/edge/src/kv-cache.tslibs/edge/src/managed.tslibs/sdk/src/esm-loader/__tests__/esm-cache.spec.tslibs/sdk/src/esm-loader/esm-cache.tslibs/sdk/src/esm-loader/esm-module-loader.tslibs/sdk/src/logger/instances/instance.file-logger.tslibs/sdk/src/server/server.instance.tslibs/utils/src/env/provider.tslibs/utils/src/storage/adapters/__tests__/cloudflare-kv-adapter.spec.tslibs/utils/src/storage/adapters/cloudflare-kv.tslibs/utils/src/storage/adapters/index.tslibs/utils/src/storage/factory.tslibs/utils/src/storage/types.tsplugins/plugin-skilled-openapi/src/__tests__/runtime-deps-injection.spec.tsplugins/plugin-skilled-openapi/src/index.tsplugins/plugin-skilled-openapi/src/skilled-openapi.plugin.ts
✅ Files skipped from review due to trivial changes (6)
- libs/utils/src/storage/adapters/index.ts
- apps/e2e/demo-e2e-cloudflare/fixture-managed/stub-empty.mjs
- plugins/plugin-skilled-openapi/src/index.ts
- CLAUDE.md
- libs/adapters/src/skills/tests/manifest-to-config.spec.ts
- apps/e2e/demo-e2e-cloudflare/project.json
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/edge/src/index.spec.ts
- libs/edge/src/managed.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/edge/src/managed.ts (1)
83-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForward
managed.cacheinto plugin options.Line 62 exposes
cacheinManagedEdgeOptions, but Line 83-90 never forwards it. Since this mapper is passed directly toSkilledOpenApiPlugin.init(...),kvBundleCacheFromEnv('BUNDLE_CACHE')is currently ignored.Suggested fix
if (managed.outbound !== undefined) options['outbound'] = managed.outbound; if (managed.bundleCacheDir !== undefined) options['bundleCacheDir'] = managed.bundleCacheDir; + if (managed.cache !== undefined) options['cache'] = managed.cache; return options;🤖 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 `@libs/edge/src/managed.ts` around lines 83 - 90, The managed.cache property is not being forwarded to the options object in the conditional block that runs lines 83-90. Add a new conditional check following the same pattern as the other properties (requireSignature, trustedKeys, dev, credentials, outbound, bundleCacheDir) to forward managed.cache into the options object when it is defined. This ensures the cache value reaches the SkilledOpenApiPlugin.init method and kvBundleCacheFromEnv is properly utilized.libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md (1)
60-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve the CJS vs ESM contradiction in Step 2/3.
Line 60-66 says Cloudflare output is CommonJS, but Line 71-78 (and the rest of this page) documents an ES Module Worker (
dist/cloudflare/index.js+nodejs_compat). These instructions conflict and will mislead users.Suggested doc correction
dist/cloudflare/ - index.js # Cloudflare Workers entry (CommonJS) — wraps your `@FrontMcp` server - main.js # Your compiled server module (CommonJS) + index.js # Cloudflare Workers entry (ES Module Worker) + main.js # Compiled app module consumed by the worker entry wrangler.toml # Wrangler configuration (overwritten on every build) -Cloudflare Workers use CommonJS (not ESM). The build command sets `--module commonjs` automatically. +Cloudflare Workers run this target as an ES Module Worker (`export default { fetch }` shape).Also applies to: 71-78
🤖 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 `@libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md` around lines 60 - 66, The documentation contains contradictory statements about module format for Cloudflare Workers deployment. Lines 60-66 claim Cloudflare Workers use CommonJS and that the build sets --module commonjs, but lines 71-78 and subsequent instructions describe an ES Module Worker setup with dist/cloudflare/index.js and nodejs_compat configuration. Determine the actual module format being used in this deployment (based on the complete build configuration and wrangler.toml) and update the statement in lines 60-66 to accurately reflect whether CommonJS or ES Modules are being used, ensuring consistency throughout the entire documentation page.
♻️ Duplicate comments (1)
libs/sdk/src/transport/web-fetch-handler.ts (1)
258-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap connect + handleRequest in try/finally to guarantee cleanup on exceptions.
If
mcpServer.connect(transport)ortransport.handleRequest()throws, themcpServerinstance (created at line 229) is never closed — a resource leak that can accumulate under error conditions. The conditional SSE/JSON teardown below only runs on the success path.🔧 Proposed fix
- await mcpServer.connect(transport); - const response = await transport.handleRequest(request, { - authInfo: options.authInfo as AuthInfo | undefined, - }); + let response: Response; + try { + await mcpServer.connect(transport); + response = await transport.handleRequest(request, { + authInfo: options.authInfo as AuthInfo | undefined, + }); + } catch (err) { + await mcpServer.close().catch(() => undefined); + throw err; + } // If the response is an SSE stream, its body keeps producing AFTER this🤖 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 `@libs/sdk/src/transport/web-fetch-handler.ts` around lines 258 - 283, The mcpServer created at line 229 is not closed if an exception is thrown during mcpServer.connect(transport) or transport.handleRequest(). Wrap the mcpServer.connect() call and the transport.handleRequest() call (along with all the subsequent response handling and conditional teardown logic) in a try/finally block to guarantee cleanup. In the finally block, ensure mcpServer is closed unless it has already been scheduled for cleanup by the SSE teardown promise. This prevents resource leaks when exceptions occur during connection or request handling.
🧹 Nitpick comments (1)
libs/sdk/src/transport/web-fetch-handler.ts (1)
141-147: 💤 Low valueConsider non-regex trailing-slash removal to silence CodeQL scanner.
The regex
/\/+$/is technically linear (no backtracking), so this is a CodeQL false positive. However, for scanner compliance and clarity, a loop-based trim is straightforward:🔧 Optional fix to satisfy static analysis
const normalizePath = (p: string): string => { const withSlash = p.startsWith('/') ? p : `/${p}`; - const trimmed = withSlash.length > 1 ? withSlash.replace(/\/+$/, '') : withSlash; + let trimmed = withSlash; + while (trimmed.length > 1 && trimmed.endsWith('/')) trimmed = trimmed.slice(0, -1); return trimmed === '' ? '/' : trimmed; };🤖 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 `@libs/sdk/src/transport/web-fetch-handler.ts` around lines 141 - 147, The regex pattern `/\/+$/` in the normalizePath function is triggering CodeQL scanner alerts. Replace the regex-based trailing slash removal in the trimmed variable assignment with a loop-based approach that iterates backwards through the string and removes consecutive trailing slashes. The loop should continue while the character at the current position is a slash and stop when it encounters a non-slash character or reaches the start of the string, then return the substring up to that point. This maintains the same functionality while satisfying static analysis requirements.Source: Linters/SAST tools
🤖 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
`@libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.md`:
- Around line 8-22: The introductory warning block (lines 8-22) clearly states
that GitHub Action, signed-resync-webhook, Durable Object stores, and Frontegg
edge auth are roadmap features not yet implemented, but subsequent sections in
the document continue to provide step-by-step instructions that appear
immediately executable. Restructure the document by adding explicit section
headers or separators that clearly delineate which workflows are "Available Now"
versus which are "Roadmap/Future", ensuring that any sections describing the
unimplemented features mentioned in the warning (GitHub Action,
signed-resync-webhook, Durable Object stores, Frontegg edge auth) are either
removed, heavily caveated, or moved to a dedicated roadmap section so users do
not attempt to follow prescriptive steps for features that do not yet exist.
---
Outside diff comments:
In `@libs/edge/src/managed.ts`:
- Around line 83-90: The managed.cache property is not being forwarded to the
options object in the conditional block that runs lines 83-90. Add a new
conditional check following the same pattern as the other properties
(requireSignature, trustedKeys, dev, credentials, outbound, bundleCacheDir) to
forward managed.cache into the options object when it is defined. This ensures
the cache value reaches the SkilledOpenApiPlugin.init method and
kvBundleCacheFromEnv is properly utilized.
In `@libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md`:
- Around line 60-66: The documentation contains contradictory statements about
module format for Cloudflare Workers deployment. Lines 60-66 claim Cloudflare
Workers use CommonJS and that the build sets --module commonjs, but lines 71-78
and subsequent instructions describe an ES Module Worker setup with
dist/cloudflare/index.js and nodejs_compat configuration. Determine the actual
module format being used in this deployment (based on the complete build
configuration and wrangler.toml) and update the statement in lines 60-66 to
accurately reflect whether CommonJS or ES Modules are being used, ensuring
consistency throughout the entire documentation page.
---
Duplicate comments:
In `@libs/sdk/src/transport/web-fetch-handler.ts`:
- Around line 258-283: The mcpServer created at line 229 is not closed if an
exception is thrown during mcpServer.connect(transport) or
transport.handleRequest(). Wrap the mcpServer.connect() call and the
transport.handleRequest() call (along with all the subsequent response handling
and conditional teardown logic) in a try/finally block to guarantee cleanup. In
the finally block, ensure mcpServer is closed unless it has already been
scheduled for cleanup by the SSE teardown promise. This prevents resource leaks
when exceptions occur during connection or request handling.
---
Nitpick comments:
In `@libs/sdk/src/transport/web-fetch-handler.ts`:
- Around line 141-147: The regex pattern `/\/+$/` in the normalizePath function
is triggering CodeQL scanner alerts. Replace the regex-based trailing slash
removal in the trimmed variable assignment with a loop-based approach that
iterates backwards through the string and removes consecutive trailing slashes.
The loop should continue while the character at the current position is a slash
and stop when it encounters a non-slash character or reaches the start of the
string, then return the substring up to that point. This maintains the same
functionality while satisfying static analysis requirements.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02cd52ba-9a49-4032-aaa6-f2b7a6a96348
⛔ Files ignored due to path filters (1)
libs/cli/src/commands/build/adapters/cloudflare.tsis excluded by!**/build/**
📒 Files selected for processing (22)
.claude/rules/flow-architecture.md.gitignoreapps/e2e/demo-e2e-cloudflare/e2e/cloudflare-worker.e2e.spec.tsapps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.tsapps/e2e/demo-e2e-cloudflare/fixture-managed/worker.tsapps/e2e/demo-e2e-cloudflare/fixture/src/main.tsdocs/frontmcp/deployment/cloudflare-worker.mdxlibs/adapters/src/skills/__tests__/manifest-to-config.spec.tslibs/adapters/src/skills/deploy/manifest-to-config.tslibs/adapters/src/skills/sources/saas-pull.source.tslibs/cli/src/commands/scaffold/create.tslibs/edge/src/index.spec.tslibs/edge/src/index.tslibs/edge/src/managed.tslibs/sdk/src/index.tslibs/sdk/src/transport/__tests__/web-fetch-handler.spec.tslibs/sdk/src/transport/index.tslibs/sdk/src/transport/web-fetch-handler.tslibs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.mdlibs/skills/catalog/frontmcp-development/references/create-plugin-hooks.mdlibs/utils/src/storage/adapters/cloudflare-kv.ts
✅ Files skipped from review due to trivial changes (3)
- libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md
- .claude/rules/flow-architecture.md
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (12)
- libs/sdk/src/index.ts
- libs/sdk/src/transport/index.ts
- apps/e2e/demo-e2e-cloudflare/fixture-managed/worker.ts
- libs/adapters/src/skills/tests/manifest-to-config.spec.ts
- libs/adapters/src/skills/deploy/manifest-to-config.ts
- libs/adapters/src/skills/sources/saas-pull.source.ts
- apps/e2e/demo-e2e-cloudflare/e2e/cloudflare-worker.e2e.spec.ts
- apps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.ts
- libs/utils/src/storage/adapters/cloudflare-kv.ts
- apps/e2e/demo-e2e-cloudflare/fixture/src/main.ts
- libs/edge/src/index.spec.ts
- libs/edge/src/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@libs/edge/src/session-host.ts`:
- Line 86: The memoization of `#scopePromise` on line 86 does not account for
promise rejection. If buildScope(this.#doEnv) rejects, the rejected promise
remains cached in `#scopePromise`, causing all subsequent calls to fail
permanently. Fix this by adding error handling to clear the `#scopePromise` cache
when the promise rejects, allowing subsequent calls to attempt initialization
again. Use a catch handler to reset `#scopePromise` to undefined before
re-throwing the error.
In `@libs/sdk/src/common/tokens/server.tokens.ts`:
- Around line 19-37: The webRequest, webCtx, and webTransport fields are
documented as web-mode-only fields but are typed as always present (required).
Mark these fields as optional in the type contract by adding the optional
property marker (?) to webRequest and webTransport, ensuring the type accurately
reflects that these fields are only available in web flows and absent in non-web
paths. This keeps the type system aligned with the documented behavior and
maintains strict TypeScript correctness.
In `@libs/sdk/src/transport/web-response.renderer.ts`:
- Around line 44-47: The hex decoding logic in the condition checking for
encoding === 'hex' lacks validation of the input payload, allowing silent
corruption through truncation of odd-length strings and coercion of invalid hex
characters to 0. Before executing the decoding loop that uses parseInt to
convert each pair of characters, add validation to ensure the body string has an
even length and contains only valid hexadecimal characters (0-9, a-f, A-F). If
validation fails, throw an appropriate error to prevent silent data corruption
rather than allowing the decoder to proceed with invalid input.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 18a1d66d-c7eb-4155-9e0f-c8005fe7a485
📒 Files selected for processing (13)
docs/frontmcp/deployment/cloudflare-worker.mdxlibs/edge/src/index.tslibs/edge/src/session-host.tslibs/sdk/src/common/schemas/http-output.schema.tslibs/sdk/src/common/tokens/server.tokens.tslibs/sdk/src/index.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/server/server.validation.tslibs/sdk/src/transport/index.tslibs/sdk/src/transport/web-fetch-handler.tslibs/sdk/src/transport/web-response.renderer.tslibs/sdk/src/transport/web-standard-mcp.tslibs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/frontmcp/deployment/cloudflare-worker.mdx
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md
added 14 commits
June 18, 2026 07:28
…on logic in tests
…on logic in tests
…d functionalities
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 by CodeRabbit
Release Notes
New Features
/healthzand MCPechotool validation.fetch/V8-isolate support, plus edge runtime support with managed auto-updating and session handling.Bug Fixes
Documentation
nodejs_compat.Chores