fix: fix circular deps in @FrontMcp #165
Conversation
…on for session recreation
📝 WalkthroughWalkthroughThe PR introduces alpha package publishing support via new Nx targets across multiple libraries, implements session initialization tracking and recreation capabilities for serverless transport contexts, refactors serverless handler lazy loading to avoid circular dependencies, enhances transport flow logging, and improves build scripts with ESM package generation and robustness. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Serverless Instance (new)<br/>Request
participant LocalTransport as LocalTransporter
participant Registry as TransportRegistry
participant Redis as Redis Session Store
participant Transport as RecreateableStreamable<br/>HTTPServerTransport
Client->>LocalTransport: handleRequest(sessionId, response)
LocalTransport->>Registry: getTransporter(sessionId)
alt Transport in memory
Registry-->>LocalTransport: existing transport
else Transport not in memory
Registry->>Redis: getStoredSession(sessionId)
alt Session found
Redis-->>Registry: storedSession {initialized: true}
Registry->>Transport: new RecreateableStreamableHTTP(options)
Registry->>Transport: setInitializationState(sessionId)
Transport->>Transport: _preInitializedSessionId = sessionId<br/>mark as initialized
Registry->>LocalTransport: markAsInitialized()
LocalTransport->>Transport: markAsInitialized()
Transport-->>LocalTransport: [ready, bypasses handshake]
else Session not found
Redis-->>Registry: null
Registry-->>LocalTransport: [initialize new session]
end
end
LocalTransport->>Transport: handleRequest(req, res, body)
Transport-->>Client: [MCP message response]
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
….json with adjusted paths
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/sdk/src/auth/session/transport-session.types.ts (1)
329-335: Add the missinginitializedfield to the schema.The schema must include the new
initializedfield to match theStoredSessioninterface and prevent validation failures.🔎 Proposed fix to add the initialized field
export const storedSessionSchema = z.object({ session: transportSessionSchema, authorizationId: z.string(), tokens: z.record(z.string(), encryptedBlobSchema).optional(), createdAt: z.number(), lastAccessedAt: z.number(), + initialized: z.boolean().optional(), });libs/uipack/src/bundler/file-cache/component-builder.ts (1)
392-399: Remove or clarify thewebpackIgnorecomment since this package builds with esbuild, not webpack.The
webpackIgnoremagic comment doesn't affect esbuild builds. The actual mechanism preventing bundling is the"external": ["esbuild"]configuration inproject.json, which tells esbuild to treatesbuildas an external dependency. The lazy loading pattern is appropriate, but the webpack-specific comment should either be removed or replaced with a comment explaining the actual esbuild external configuration that handles this.
♻️ Duplicate comments (4)
libs/json-schema-to-zod-v3/project.json (1)
66-72: Duplicate: Same publish-alpha implementation as other libraries.This publish-alpha target is identical to the ones in libs/mcp-from-openapi, libs/plugins, libs/cli, and libs/testing. Please see the review comment on libs/mcp-from-openapi/project.json for a detailed analysis and refactoring recommendation to extract this logic into a shared script.
libs/plugins/project.json (1)
84-90: Duplicate: Same publish-alpha implementation as other libraries.This publish-alpha target is identical to those in libs/mcp-from-openapi, libs/json-schema-to-zod-v3, libs/cli, and libs/testing. Please see the review comment on libs/mcp-from-openapi/project.json for the refactoring recommendation.
libs/cli/project.json (1)
54-60: Duplicate: Same publish-alpha implementation as other libraries.This publish-alpha target is identical to those in other library project.json files. Please see the review comment on libs/mcp-from-openapi/project.json for the detailed analysis and refactoring recommendation.
libs/testing/project.json (1)
105-111: Duplicate: Same publish-alpha implementation as other libraries.This publish-alpha target duplicates the implementation from other libraries. Please see the review comment on libs/mcp-from-openapi/project.json for the refactoring recommendation to extract this into a shared script.
🧹 Nitpick comments (5)
scripts/strip-dist-from-pkg.js (1)
68-68: Usepath.join()for consistent cross-platform path handling.String concatenation with
/can cause issues on Windows wherepath.dirname()returns backslash separators. This is inconsistent with line 116 which correctly usespath.join().🔎 Proposed fix
-const esmDir = path.dirname(pkgPath) + '/esm'; +const esmDir = path.join(path.dirname(pkgPath), 'esm');libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
15-24: Consider simplifying the type union.Since
RecreateableStreamableHTTPServerTransportextendsStreamableHTTPServerTransport, including both in the union creates redundancy. A value of typeRecreateableStreamableHTTPServerTransportis already assignable toStreamableHTTPServerTransport.However, keeping both explicit is acceptable for documentation purposes and makes the supported types clear to readers. The JSDoc comment accurately explains the relationship.
🔎 Optional: Simplified union (if desired)
-export type SupportedTransport = - | StreamableHTTPServerTransport - | SSEServerTransport - | RecreateableStreamableHTTPServerTransport; +export type SupportedTransport = + | RecreateableStreamableHTTPServerTransport + | SSEServerTransport;This would still accept
StreamableHTTPServerTransportdue to structural typing, though the explicit listing may be clearer.libs/sdk/src/transport/adapters/streamable-http-transport.ts (3)
34-35: Avoidanytype - useunknownor extract proper type.Per coding guidelines,
anyshould be avoided without strong justification. The comment mentions avoiding "complex type extraction," but this could be improved.🔎 Suggested improvement
/** * Event store for resumability support. - * Uses any to avoid complex type extraction from MCP SDK's optional options type. */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - eventStore?: any; + eventStore?: unknown;Alternatively, if the MCP SDK exports the event store type, import and use it directly.
60-60: Unused private field_preInitializedSessionId.This field is assigned in
setInitializationStatebut never read. If it's for future debugging or tracking purposes, consider adding a getter or documenting its intended use.🔎 Options
- Remove if unused:
- private _preInitializedSessionId: string | undefined;
- Or add a getter if useful for debugging:
+ get preInitializedSessionId(): string | undefined { + return this._preInitializedSessionId; + }
102-115: Consider removing no-op method overrides.Both
handleRequestandsendsimply delegate tosuperwithout any additional logic. The JSDoc comments suggest they exist for "session recreation scenario" handling, but no actual handling is implemented.If these are placeholders for future logic, consider adding a TODO comment. Otherwise, they can be removed since TypeScript inheritance already provides this behavior.
🔎 Suggested cleanup if no additional logic is needed
- /** - * Override handleRequest to handle session recreation scenario. - * When we've set initialization state manually, we need to ensure - * the transport operates correctly. - */ - override async handleRequest( - req: AuthenticatedServerRequest, - res: ServerResponse, - parsedBody?: unknown, - ): Promise<void> { - return super.handleRequest(req, res, parsedBody); - } - - /** - * Sends a JSON-RPC message through the transport. - */ - override async send(message: JSONRPCMessage, options?: { relatedRequestId?: RequestId }): Promise<void> { - return super.send(message, options); - }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
libs/cli/src/commands/build/adapters/vercel.tsis excluded by!**/build/**libs/cli/src/commands/build/bundler.tsis excluded by!**/build/**libs/uipack/src/build/builders/base-builder.tsis excluded by!**/build/**
📒 Files selected for processing (26)
.gitignoreapps/demo/src/main.tslibs/adapters/project.jsonlibs/cli/project.jsonlibs/json-schema-to-zod-v3/project.jsonlibs/mcp-from-openapi/project.jsonlibs/plugins/project.jsonlibs/sdk/project.jsonlibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/transport/__tests__/transport.registry.test.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/transport/transport.registry.tslibs/sdk/src/transport/transport.remote.tslibs/sdk/src/transport/transport.types.tslibs/testing/project.jsonlibs/ui/project.jsonlibs/uipack/project.jsonlibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.tsscripts/strip-dist-from-pkg.js
🧰 Additional context used
📓 Path-based instructions (9)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use type parameters with constraints instead of unconstrained generics
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/transport/transport.types.tslibs/sdk/src/transport/transport.remote.tsapps/demo/src/main.tslibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/transport/transport.registry.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/transport/__tests__/transport.registry.test.ts
libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/src/**/*.ts: MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead ofunknown
Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()method for dynamic capability exposure instead of hardcoding capabilities
UsechangeScopeproperty instead ofscopein change events to avoid confusion with Scope class
Validate URIs per RFC 3986 at metadata level using proper validation
Validate hook flows and fail fast on invalid hook configurations with specific error messages
Files:
libs/sdk/src/transport/transport.types.tslibs/sdk/src/transport/transport.remote.tslibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/transport/transport.registry.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/transport/__tests__/transport.registry.test.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/transport/transport.types.tslibs/sdk/src/transport/transport.remote.tslibs/ui/project.jsonlibs/plugins/project.jsonlibs/sdk/src/auth/session/transport-session.types.tslibs/mcp-from-openapi/project.jsonlibs/uipack/project.jsonlibs/sdk/src/transport/transport.registry.tslibs/adapters/project.jsonlibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/project.jsonlibs/cli/project.jsonlibs/sdk/src/scope/flows/http.request.flow.tslibs/json-schema-to-zod-v3/project.jsonlibs/testing/project.jsonlibs/uipack/src/bundler/file-cache/component-builder.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/transport/__tests__/transport.registry.test.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/main.ts
libs/uipack/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (libs/uipack/CLAUDE.md)
libs/uipack/src/**/*.{ts,tsx,js,jsx}: No React dependency in @frontmcp/uipack - use @frontmcp/ui for React components and hooks
Always validate component options before using them
Do not expose internal error details to users - sanitize error messages
Files:
libs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
libs/uipack/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (libs/uipack/CLAUDE.md)
libs/uipack/src/**/*.{ts,tsx}: Use.strict()on all Zod schemas for validation
Avoid usinganytype without justification in TypeScript
Files:
libs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
libs/uipack/src/{theme,build,bundler,base-template}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (libs/uipack/CLAUDE.md)
Use theme.cdn configuration instead of hard-coding CDN URLs
Files:
libs/uipack/src/bundler/file-cache/component-builder.ts
libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (libs/uipack/CLAUDE.md)
Properly escape user-provided content to prevent XSS attacks in HTML generation
Files:
libs/uipack/src/renderers/utils/transpiler.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including error cases and constructor validation
Include error classinstanceofchecks in tests to validate error handling
Files:
libs/sdk/src/transport/__tests__/transport.registry.test.ts
🧠 Learnings (16)
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Use Nx for build system with commands like `nx build sdk`, `nx test`, `nx run-many -t test`
Applied to files:
libs/ui/project.jsonlibs/plugins/project.jsonlibs/mcp-from-openapi/project.jsonlibs/uipack/project.jsonlibs/adapters/project.jsonlibs/sdk/project.jsonlibs/cli/project.jsonlibs/testing/project.json
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to libs/sdk/src/**/*.ts : MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead of `unknown`
Applied to files:
libs/sdk/src/auth/session/transport-session.types.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/common/decorators/front-mcp.decorator.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to libs/*/package.json : Each library in /libs/* is independent and publishable under frontmcp/* scope
Applied to files:
libs/mcp-from-openapi/project.json.gitignore
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/{adapters,theme,build}/**/*.test.{ts,tsx,js,jsx} : Test component behavior across all platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/uipack/project.json.gitignore
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.test.{ts,tsx,js,jsx} : Test HTML escaping for user-provided content in security-critical paths
Applied to files:
libs/uipack/project.json.gitignorelibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:41.807Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.807Z
Learning: Applies to libs/ui/src/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/sdk/src/common/decorators/front-mcp.decorator.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/{theme,build,bundler,base-template}/**/*.{ts,tsx,js,jsx} : Use theme.cdn configuration instead of hard-coding CDN URLs
Applied to files:
.gitignorelibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/{build,base-template,adapters}/**/*.{ts,tsx,js,jsx} : Use buildCdnScriptsFromTheme with appropriate options (inline: true for blocked-network platforms like Claude)
Applied to files:
.gitignorelibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
.gitignorelibs/uipack/src/bundler/file-cache/component-builder.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : No React dependency in frontmcp/uipack - use frontmcp/ui for React components and hooks
Applied to files:
.gitignorelibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to libs/sdk/src/**/index.ts : Export public API through barrel files - export users' needed items, no legacy exports with different names
Applied to files:
.gitignorelibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:41.807Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.807Z
Learning: Applies to libs/ui/src/bundler/**/*.{ts,tsx} : The bundler module must re-export utilities from frontmcp/uipack/bundler and provide SSR component bundling functionality
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/*/index.ts : Implement entry point re-exports in index.ts files for each module (adapters, build, bundler, etc.)
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.{ts,tsx,js,jsx} : Properly escape user-provided content to prevent XSS attacks in HTML generation
Applied to files:
libs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.384Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.384Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Avoid using `any` type without justification in TypeScript
Applied to files:
libs/uipack/src/renderers/utils/transpiler.ts
🧬 Code graph analysis (6)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
libs/testing/src/client/mcp-test-client.ts (1)
sessionId(207-209)
libs/sdk/src/transport/adapters/transport.streamable-http.adapter.ts (1)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
RecreateableStreamableHTTPServerTransport(59-116)
libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
RecreateableStreamableHTTPServerTransport(59-116)
libs/sdk/src/scope/flows/http.request.flow.ts (4)
libs/sdk/src/common/interfaces/internal/primary-auth-provider.interface.ts (1)
transport(84-86)libs/sdk/src/context/frontmcp-context.ts (1)
transport(271-278)libs/testing/src/client/mcp-test-client.ts (1)
request(823-872)libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
decideIntent(396-450)
libs/sdk/src/transport/flows/handle.streamable-http.flow.ts (2)
libs/testing/src/interceptor/interceptor-chain.ts (1)
logger(185-190)libs/testing/src/client/mcp-test-client.ts (1)
session(211-227)
libs/sdk/src/transport/transport.local.ts (1)
libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
SupportedTransport(20-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: E2E Tests (demo-e2e-hooks)
- GitHub Check: E2E Tests (demo-e2e-ui)
- GitHub Check: E2E Tests (demo-e2e-errors)
- GitHub Check: E2E Tests (demo-e2e-serverless)
- GitHub Check: E2E Tests (demo-e2e-multiapp)
- GitHub Check: E2E Tests (demo-e2e-cache)
- GitHub Check: E2E Tests (demo-e2e-notifications)
- GitHub Check: E2E Tests (demo-e2e-codecall)
- GitHub Check: E2E Tests (demo-e2e-providers)
- GitHub Check: E2E Tests (demo-e2e-redis)
- GitHub Check: E2E Tests (demo-e2e-openapi)
- GitHub Check: E2E Tests (demo-e2e-transparent)
- GitHub Check: E2E Tests (demo-e2e-public)
- GitHub Check: Unit Tests
🔇 Additional comments (24)
.gitignore (1)
64-67: Configuration entries appropriately support alpha-publish and build workflows.The new ignore entries align well with the PR's alpha-publish targets and multi-library tsbuildinfo generation. The
.npmrc.verdaccioentry prevents committing local registry credentials,**/.script.localallows per-script local development overrides, and the broadened**/tsconfig.lib.tsbuildinfopattern accommodates the expanded build artifact footprint across the monorepo.scripts/strip-dist-from-pkg.js (5)
6-17: LGTM!Solid error handling with clear, user-friendly error messages for missing files and parse failures.
19-36: LGTM!Clean recursive implementation that properly handles nested structures, prunes empty values, and strips the
./dist/prefix at leaf nodes.
38-48: LGTM!Proper handling of top-level fields and exports map with appropriate cleanup when the result is empty.
50-65: LGTM!Consistent handling of the
binmap and robust error handling for the file write operation.
69-124: LGTM!The ESM package.json generation logic is well-structured. The path adjustments correctly handle the relative path transformations needed when the package.json lives inside the
esm/subdirectory.libs/uipack/src/renderers/utils/transpiler.ts (1)
52-63: Consistent lazy loading pattern to prevent circular dependencies.This follows the same pattern as
component-builder.ts, usingwebpackIgnore: trueto prevent build-time bundler processing. The error handling is appropriately lenient here (warning + return null) since@swc/coreis an optional dependency, unlike the requiredesbuildin the other file.libs/sdk/src/transport/__tests__/transport.registry.test.ts (1)
18-18: LGTM!The mock correctly includes the new
markAsInitializedmethod to satisfy the updatedTransporterinterface contract. This ensures tests remain valid after the interface change.libs/sdk/src/transport/transport.types.ts (1)
63-69: LGTM!The new
markAsInitialized()method is a clean interface addition with clear documentation explaining its purpose for session recreation scenarios. The synchronousvoidreturn type is appropriate for a state-setting operation.apps/demo/src/main.ts (1)
18-20: LGTM!The transport configuration is correctly restructured into a nested
transportblock withinauth, aligning with the updated SDK that reads transport config fromscope.metadata.transportorscope.auth.transport. This demo serves as a good reference for the new configuration structure.libs/ui/project.json (1)
96-103: Consistent publish-alpha workflow added.The target follows the same pattern as other libraries in this PR, enabling alpha-tagged publishing to Verdaccio. The shell pipeline correctly handles version manipulation and silent fallback for missing
.npmrc.Consider whether the hardcoded Verdaccio URL (
https://verdaccio.ngrok.app) should be environment-configurable for different deployment scenarios.libs/uipack/project.json (1)
115-122: LGTM!Consistent publish-alpha target matching the pattern established across other libraries in this PR.
libs/adapters/project.json (1)
82-89: LGTM!Consistent publish-alpha target matching the established pattern across libraries.
libs/sdk/project.json (1)
95-102: LGTM!Consistent publish-alpha target for the SDK, enabling alpha testing of the transport and session recreation changes introduced in this PR.
libs/sdk/src/scope/flows/http.request.flow.ts (1)
207-221: LGTM! Clean fallback pattern with helpful debugging.The transport config resolution correctly implements backward compatibility by falling back from
scope.metadata.transporttoscope.auth.transport. Both sources are guaranteed to provide values:scope.metadata.transportdefaults to an empty object parsed through the schema, andscope.auth.transporthas a getter that returnsthis.options.transport ?? DEFAULT_TRANSPORT_CONFIG. The debug logging provides valuable visibility into transport decisions without exposing sensitive data.libs/sdk/src/common/decorators/front-mcp.decorator.ts (2)
7-41: LGTM! Circular dependency fix is well-implemented.The lazy loading pattern successfully avoids the circular dependency with the SDK entry point by using direct relative imports. The
ServerlessHandlerFnstype appropriately usesunknownfor handler parameters (adhering to strict TypeScript guidelines), and error handling is consistent with the existing pattern.
88-91: Clean refactor from dynamic require to direct imports.The change from
require('@frontmcp/sdk')togetServerlessHandlerFns()maintains the same behavior while eliminating the circular dependency. The destructuring assignment clearly documents the three handler functions being imported.libs/sdk/src/transport/transport.registry.ts (2)
292-298: Well-implemented session initialization tracking.The initialization state propagation correctly handles session recreation by:
- Checking
storedSession.initialized !== false(backwards compatible with old sessions)- Calling
markAsInitialized()to set the MCP SDK's internal flag- Preventing subsequent requests from being rejected as uninitialized
The backwards compatibility strategy is sound—treating missing
initializedfields astrueensures old sessions continue to work.
400-400: Correct initialization flag for new sessions.Setting
initialized: truewhen persisting new streamable-http sessions ensures that recreated sessions will properly reflect their initialization state. This pairs correctly with the recreation logic at lines 292-298.libs/sdk/src/transport/transport.remote.ts (1)
31-33: Appropriate no-op implementation for remote transporter.The no-op
markAsInitialized()method correctly implements theTransporterinterface while acknowledging that initialization state is managed on the remote node. The comment clearly explains the rationale.libs/sdk/src/transport/flows/handle.streamable-http.flow.ts (1)
217-250: LGTM - Enhanced logging for transport flow debugging.The added logging statements provide good observability for the session recreation flow. The session ID truncation (
.slice(0, 20)) is consistent and avoids logging full session identifiers. Log levels are appropriate (infofor normal flow,warnfor failures).libs/sdk/src/transport/transport.local.ts (1)
69-76: LGTM - Clean delegation pattern for session initialization.The
markAsInitialized()method properly delegates to the adapter with clear documentation explaining its purpose for Redis session recreation. This follows the established pattern in the class where public methods delegate to the adapter.libs/sdk/src/transport/adapters/transport.streamable-http.adapter.ts (1)
141-147: LGTM - Proper override ofmarkAsInitializedwith helpful logging.The override correctly uses the public
setInitializationStateAPI fromRecreateableStreamableHTTPServerTransport, avoiding direct access to private properties. The logging confirms the operation succeeded by checkingisInitialized.Minor observation:
this.key.sessionIdis set in the constructor and shouldn't be undefined, so the optional chaining (?.slice) on line 144 is defensive but unnecessary. This is fine to keep for safety.libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
58-64: LGTM - Good use of template method pattern.The default no-op implementation allows subclasses to opt-in to initialization marking. This is appropriate since only
TransportStreamableHttpAdapterneeds this functionality for session recreation, while SSE transports don't require it.
| "publish-alpha": { | ||
| "executor": "nx:run-commands", | ||
| "dependsOn": ["build"], | ||
| "options": { | ||
| "command": "cp .npmrc.verdaccio {projectRoot}/dist/.npmrc 2>/dev/null || true && ts=${ALPHA_TS:-$(date +%s)} && base=$(jq -r '.version' {projectRoot}/dist/package.json | sed 's/-.*//') && jq --arg v \"$base-alpha.$ts\" '.version=$v' {projectRoot}/dist/package.json > {projectRoot}/dist/package.tmp.json && mv {projectRoot}/dist/package.tmp.json {projectRoot}/dist/package.json && cd {projectRoot}/dist && npm publish --access public --tag alpha --registry=\"https://verdaccio.ngrok.app\"", | ||
| "cwd": "{workspaceRoot}" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
find libs -name "project.json" -type f | head -20Repository: agentfront/frontmcp
Length of output: 306
🏁 Script executed:
rg "publish-alpha" libs/ --type json -A 10Repository: agentfront/frontmcp
Length of output: 8714
🌐 Web query:
ngrok production use stability temporary tunnels
💡 Result:
Short answer
- ngrok can be used in production, but “temporary” tunnels (free/dev behaviour) are less stable and not designed for mission‑critical traffic — use paid/enterprise features (reserved domains, BYO domain, dedicated endpoints, higher limits and SLAs) or a self‑hosted alternative for production reliability. [1][2]
Key points and practical guidance
- Free/dev tunnels: limited bandwidth, request/endpoint limits and ephemeral dev domains; they can be be interrupted or rate‑limited and are not intended as a production-grade public ingress. Use paid plans to remove many limits. [1][3]
- Production features: paid and Pay‑as‑you‑go plans offer reserved/custom domains, unlimited endpoints (with paid usage), stronger quotas, identity/SSO, and enterprise options (on‑prem/BYOD and SLAs). If you need stable public URLs, persistent endpoints or dedicated IPs, buy the relevant plan. [1][2]
- Security/compliance: traffic traverses ngrok infrastructure by design — review contracts/BAAs and logging policies if you handle regulated or sensitive data. Consider on‑prem/self‑hosted if you must avoid third‑party routing. [1][4]
- Self‑hosted / alternatives: mature open‑source/self‑hosted options exist (frp, chisel, Octelium, Cloudflare Tunnel alternatives, etc.) if you need full control, predictable uptime or to avoid third‑party routing. Those require running and operating server infrastructure. [4][5]
- Operational best practices: use reserved domains/BYO domains for stable URLs; run the ngrok agent as a managed service; add health checks and failover (DNS, load balancer, or secondary ingress); monitor logs/usage and automate reconnection; limit exposure with auth, IP allowlists or mTLS. [1][2]
If you want, I can:
- compare ngrok paid tiers vs specific self‑hosted options for your workload (traffic, SLA needs, compliance), or
- provide a short playbook (systemd/docker service + healthchecks + fallback DNS) to run tunnels reliably in production.
Extract duplicated publish-alpha logic to a shared script.
This complex shell command is duplicated across 8 library project.json files (adapters, cli, json-schema-to-zod-v3, mcp-from-openapi, plugins, sdk, testing, ui, uipack). The command has multiple issues:
- DRY violation: Identical logic repeated across 8 files makes maintenance error-prone
- No error handling: The command chain can silently fail at multiple points
- Tool dependencies: Assumes
jq,sed, anddateare available without validation - Unstable production registry: The Verdaccio URL uses
https://verdaccio.ngrok.app, a free ngrok tunnel. Free/dev ngrok tunnels are ephemeral and not intended for production use—they can be interrupted or rate-limited and lack SLA guarantees. Production use requires ngrok paid plans with reserved domains or a self-hosted alternative.
Extract the logic to scripts/publish-alpha.sh with proper error handling and validation, then update all 8 project.json files to call this shared script instead.
🤖 Prompt for AI Agents
In libs/mcp-from-openapi/project.json around lines 66 to 72, the long shell
command for "publish-alpha" is duplicated and brittle; extract that logic into a
new executable script scripts/publish-alpha.sh, move the complex command there,
and replace the project.json command with a simple invocation of that script; in
the script add strict error handling (set -euo pipefail), validate required
tools (jq, sed, date, npm) before running, read registry and alpha timestamp
from environment variables (with sane defaults), copy .npmrc if present, compute
base version and bump to base-alpha.$TS via jq safely, fail early on any command
error and emit clear error messages, and update the other 7 project.json files
to call the shared scripts/publish-alpha.sh so the logic is DRY and robust.
| /** Whether the MCP protocol initialization handshake was completed */ | ||
| initialized?: boolean; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n libs/sdk/src/auth/session/transport-session.types.ts | head -400Repository: agentfront/frontmcp
Length of output: 12229
🏁 Script executed:
#!/bin/bash
# Search for storedSessionSchema usage across the codebase
rg -nP --type=ts 'storedSessionSchema' -C3Repository: agentfront/frontmcp
Length of output: 3299
Critical: Zod schema missing the initialized field.
The initialized field was added to the StoredSession interface (line 165) but the corresponding storedSessionSchema (lines 329-335) was not updated. This causes the schema to silently strip the field during validation when sessions are retrieved from Redis or KV stores, resulting in data loss.
Update storedSessionSchema to include:
initialized: z.boolean().optional(),
🤖 Prompt for AI Agents
In libs/sdk/src/auth/session/transport-session.types.ts around lines 164-165 and
the storedSessionSchema at ~329-335, the StoredSession interface now includes an
optional initialized field but the Zod schema omits it, causing the field to be
stripped during validation; update the storedSessionSchema to include
initialized: z.boolean().optional(), place it next to the other optional boolean
fields, and run type-checks/tests to ensure the schema accepts and preserves the
field when parsing/storing sessions.
| get isInitialized(): boolean { | ||
| // Access the internal WebStandardTransport's _initialized flag | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return (this as any)._webStandardTransport?._initialized ?? false; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the transport to an initialized state with the given session ID. | ||
| * Use this when recreating a transport from a stored session. | ||
| * | ||
| * This method allows you to "restore" a session without replaying the | ||
| * initialization handshake. After calling this method, the transport | ||
| * will accept requests with the given session ID. | ||
| * | ||
| * @param sessionId - The session ID that was previously assigned to this session | ||
| */ | ||
| setInitializationState(sessionId: string): void { | ||
| this._preInitializedSessionId = sessionId; | ||
|
|
||
| // Access the internal WebStandardTransport and set both flags | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const webTransport = (this as any)._webStandardTransport; | ||
| if (webTransport) { | ||
| webTransport._initialized = true; | ||
| webTransport.sessionId = sessionId; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@modelcontextprotocol/sdk public API transport initialization session restoration
💡 Result:
Brief how-to (TypeScript SDK):
-
Server: use StreamableHTTPServerTransport with a sessionIdGenerator and onsessioninitialized; store the transport keyed by sessionId and reuse it when a request arrives with that session id. Clean up when transport.onclose fires. Example pattern: check req.headers['mcp-session-id'] → reuse transport; if not present and request is an initialize request, create a new transport which will emit its session id via onsessioninitialized. [1][2]
-
Client: when reconnecting include the MCP session id in the request header (mcp-session-id) so the server can find and reattach the existing transport; the transport.handleRequest(req,res,body) call then resumes that session. The SDK provides helpers (e.g., isInitializeRequest) to detect initialization requests. [1][2]
-
Implementation notes:
- Session ID can be generated (randomUUID) or supplied by your session store. [2]
- Persist session IDs on the client side (or in cookies) so reconnect requests include mcp-session-id.
- Ensure transport cleanup removes the stored transport to avoid leaks (transport.onclose). [2][3]
References:
[1] modelcontextprotocol/typescript-sdk — Streamable HTTP example (server + session management).
[2] @modelcontextprotocol/sdk npm README and examples (Streamable HTTP, headers, initialization).
[3] modelcontextprotocol/typescript-sdk repository overview and docs.
🏁 Script executed:
# Find package.json or lock files to check SDK version constraints
fd -e "package.json" -e "package-lock.json" -e "pnpm-lock.yaml" | head -5Repository: agentfront/frontmcp
Length of output: 45
🏁 Script executed:
# Examine the actual file to understand context and imports
head -100 libs/sdk/src/transport/adapters/streamable-http-transport.tsRepository: agentfront/frontmcp
Length of output: 3891
Accessing private SDK internals for session restoration—consider using standard session management pattern instead.
The code directly accesses _webStandardTransport._initialized and webTransport.sessionId (private implementation details). While this enables session recreation, the MCP SDK's documented pattern for serverless environments is to:
- Store initialized transports by session ID using
sessionIdGeneratorand theonsessioninitializedcallback - Reuse stored transports when reconnection requests include the
mcp-session-idheader
This approach avoids accessing private properties entirely. If this custom setInitializationState() method is necessary for your use case, document why the standard pattern is insufficient and add a constraint to the SDK version dependency (currently only documented in file comments).
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.