fix: fix circular deps in @FrontMcp #165
Conversation
…on for session recreation
📝 WalkthroughWalkthroughAdds per-package "publish-alpha" Nx targets, session-initialization tracking and transport recreation APIs, serverless handler lazy-loading refactor, expanded transport logging and persistence, build/script improvements including ESM package.json generation, and small .gitignore updates. (49 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client / Request
participant Local as LocalTransporter
participant Registry as TransportRegistry
participant Redis as RedisStore
participant Transport as RecreateableTransport
Client->>Local: incoming request (sessionId)
Local->>Registry: getTransporter(sessionId)
alt transport in-memory
Registry-->>Local: existing transporter
else not in-memory
Registry->>Redis: getStoredSession(sessionId)
alt storedSession found
Redis-->>Registry: storedSession { initialized: true? }
Registry->>Transport: create RecreateableTransport from storedSession
Transport->>Transport: setInitializationState(sessionId)
Transport-->>Registry: initialized
Registry->>Local: transporter ready (pre-initialized)
else no stored session
Redis-->>Registry: null
Registry-->>Local: initialize new transporter (handshake)
end
end
Local->>Transport: handleRequest(req, res)
Transport-->>Client: respond (MCP messages / events)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 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
|
| Status | Scanner |  Critical |
 High |
 Medium |
 Low |
Total (0) |
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details |
💻 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.
…initialization for session recreation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
80-88: Accessing private SDK internals—version constraint recommended.This duplicates the concern from the past review. The code accesses
_webStandardTransport._initializedandsessionIdwhich are private implementation details of@modelcontextprotocol/sdk. This approach works but could break on SDK updates.Recommendations:
- Pin or constrain the SDK version in
package.jsonto prevent unexpected breakage- Add a comment noting the specific SDK version this was tested against
- Consider adding a runtime safety check
#!/bin/bash # Check if SDK version is pinned in package.json cat libs/sdk/package.json | jq '.dependencies["@modelcontextprotocol/sdk"]'
🧹 Nitpick comments (4)
scripts/strip-dist-from-pkg.js (1)
38-42: Consider more explicit null/undefined checks.While stripDist handles non-string values correctly, the code could be more explicit about null/undefined handling for clarity:
-if (pkg.main) pkg.main = stripDist(pkg.main); -if (pkg.types) pkg.types = stripDist(pkg.types); -if (pkg.module) pkg.module = stripDist(pkg.module); +if (pkg.main != null) pkg.main = stripDist(pkg.main); +if (pkg.types != null) pkg.types = stripDist(pkg.types); +if (pkg.module != null) pkg.module = stripDist(pkg.module);This makes the intent clearer that we're checking for null/undefined, not just falsy values.
libs/sdk/src/transport/adapters/transport.sse.adapter.ts (1)
14-54: Consider extracting common transport setup logic to reduce duplication.Both
createTransportandcreateTransportFromSessionshare identical error/close handler setup (lines 21-26 and 49-52). This duplication could be reduced with a helper method.🔎 Suggested refactor
+ private setupTransportHandlers(transport: RecreateableSSEServerTransport): void { + transport.onerror = (error) => { + console.error('SSE error:', error instanceof Error ? error.message : 'Unknown error'); + }; + transport.onclose = this.destroy.bind(this); + } + override createTransport(sessionId: string, res: ServerResponse): RecreateableSSEServerTransport { this.sessionId = sessionId; this.logger.info(`new transport session: ${sessionId.slice(0, 40)}`); const scopePath = this.scope.fullPath; const transport = new RecreateableSSEServerTransport(`${scopePath}/message`, res, { sessionId: sessionId, }); - transport.onerror = (error) => { - // Use safe logging to avoid Node.js 24 util.inspect bug with Zod errors - console.error('SSE error:', error instanceof Error ? error.message : 'Unknown error'); - }; - transport.onclose = this.destroy.bind(this); + this.setupTransportHandlers(transport); return transport; } createTransportFromSession( sessionId: string, res: ServerResponse, lastEventId?: number, ): RecreateableSSEServerTransport { this.sessionId = sessionId; this.logger.info(`recreating transport session: ${sessionId.slice(0, 40)}, lastEventId: ${lastEventId ?? 'none'}`); const scopePath = this.scope.fullPath; const transport = new RecreateableSSEServerTransport(`${scopePath}/message`, res, { sessionId: sessionId, initialEventId: lastEventId, }); - transport.onerror = (error) => { - console.error('SSE error:', error instanceof Error ? error.message : 'Unknown error'); - }; - transport.onclose = this.destroy.bind(this); + this.setupTransportHandlers(transport); return transport; }libs/sdk/src/transport/adapters/sse-transport.ts (1)
82-98: Session ID mismatch warning may mask bugs—consider stronger handling.The
setSessionStatemethod logs a warning whensessionIddoesn't match but silently continues. While defensive, this could mask bugs in session restoration logic where mismatched IDs indicate a serious state corruption.Consider either:
- Throwing an error for mismatches (stricter, safer)
- Using
console.errorinstead ofconsole.warnfor better visibility- Adding a return value to indicate success/failure
libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
16-27: SupportedTransport union has redundant types due to inheritance.Since
RecreateableStreamableHTTPServerTransport extends StreamableHTTPServerTransportandRecreateableSSEServerTransport extends SSEServerTransport, the union could be simplified:export type SupportedTransport = | StreamableHTTPServerTransport | SSEServerTransport;The recreateable variants are already assignable to their base types. However, keeping them explicit documents intent and aids discoverability. This is a minor observation—current implementation is correct.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
libs/sdk/package.jsonlibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/transport/adapters/sse-transport.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/index.tslibs/uipack/src/bundler/file-cache/component-builder.tsscripts/strip-dist-from-pkg.js
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/uipack/src/bundler/file-cache/component-builder.ts
- libs/sdk/src/auth/session/transport-session.types.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/index.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/transport/adapters/sse-transport.ts
libs/sdk/src/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export public API through barrel files - export users' needed items, no legacy exports with different names
Files:
libs/sdk/src/transport/index.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/index.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/transport/adapters/sse-transport.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/index.tslibs/sdk/package.jsonlibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/transport/adapters/sse-transport.ts
libs/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Each library in /libs/* is independent and publishable under @frontmcp/* scope
Files:
libs/sdk/package.json
🧠 Learnings (6)
📓 Common learnings
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`
📚 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:
libs/sdk/src/transport/index.tslibs/sdk/package.json
📚 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/sdk/src/transport/index.tslibs/sdk/package.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/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/transport/index.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/sdk/src/transport/index.tslibs/sdk/package.json
📚 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/package.json : Entry points must match the documented paths: frontmcp/ui/react, frontmcp/ui/renderers, frontmcp/ui/render, frontmcp/ui/universal, frontmcp/ui/bundler, frontmcp/ui/bridge, frontmcp/ui/components, frontmcp/ui/layouts, frontmcp/ui/web-components
Applied to files:
libs/sdk/package.json
🧬 Code graph analysis (3)
libs/sdk/src/transport/adapters/transport.sse.adapter.ts (2)
libs/sdk/src/transport/adapters/sse-transport.ts (1)
RecreateableSSEServerTransport(34-99)libs/sdk/src/transport/index.ts (1)
RecreateableSSEServerTransport(25-25)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (2)
libs/sdk/src/transport/index.ts (2)
StreamableHTTPServerTransportOptions(22-22)RecreateableStreamableHTTPServerTransport(21-21)libs/testing/src/client/mcp-test-client.ts (1)
sessionId(207-209)
libs/sdk/src/transport/adapters/sse-transport.ts (2)
libs/sdk/src/transport/index.ts (4)
RecreateableSSEServerTransportOptions(25-25)SSEServerTransportOptions(28-28)RecreateableSSEServerTransport(25-25)SSEServerTransport(28-28)libs/testing/src/client/mcp-test-client.ts (1)
sessionId(207-209)
🔇 Additional comments (12)
scripts/strip-dist-from-pkg.js (6)
6-17: LGTM!The error handling for file existence and JSON parsing is clear and appropriate for a build script.
19-19: LGTM!The stripDist function correctly handles both string and non-string values.
21-36: LGTM!The walk function correctly handles recursive traversal, pruning empty values, and removing "development" conditions while applying the stripDist transformation to leaf nodes.
44-55: LGTM!The exports and bin map cleaning logic is consistent and correctly handles empty results by deleting the fields.
59-65: LGTM!The file write operation has appropriate error handling and clear success/error messaging.
76-87: The path adjustment logic in theadjustForEsmfunction correctly handles the expected package structure. AfterstripDistremoves the./dist/prefix, the logic properly transforms paths:
.mjsfiles (ESM modules already at./esm/index.mjs) become relative to the esm/ subdirectory (./index.mjs)- CJS/types files (at
./index.js) get../prepended to reference the parent dist/ directory (../index.js)This transformation is correct for the monorepo's build structure where CJS is built to
dist/and ESM todist/esm/.libs/sdk/package.json (1)
55-65: LGTM! New transport subpath export is well-structured.The new
./transportexport follows the established pattern with properdevelopment,require, andimportentries. This correctly exposes the transport barrel for consumers to import recreateable transport classes.libs/sdk/src/transport/index.ts (1)
1-31: Well-documented barrel file with clear API surface.The barrel file correctly exports the public transport API with:
- Clear JSDoc documentation and usage example
- Logical grouping by purpose (recreateable transports, legacy compatibility, types)
- Exports aligned with coding guidelines for barrel files
As per coding guidelines, this properly exports users' needed items without legacy exports with different names.
libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
61-67: LGTM! Proper template method pattern for session initialization.The
markAsInitialized()method provides a clean extension point for subclasses that need to set the MCP SDK's_initializedflag during session recreation. The default no-op is appropriate for the base class.libs/sdk/src/transport/adapters/streamable-http-transport.ts (2)
28-32: Justifiedanyfor eventStore type.The comment explains this avoids complex type extraction from MCP SDK's optional options type. Per coding guidelines,
anywith strong justification is acceptable. Consider usingunknownwith a runtime check if stricter typing is desired later.
64-68: Defensive getter with proper fallback.The optional chaining (
?.) and?? falsefallback provide safe access to the private internal state, handling cases where_webStandardTransportmight not exist.libs/sdk/src/transport/adapters/sse-transport.ts (1)
69-73: Justifiedanycast for accessing parent's private field.The
eslint-disableis appropriately scoped and the cast is necessary to restore the_eventIdCounterstate from the parent class during session recreation. The parent classSSEServerTransport(defined inlegacy.sse.tranporter.ts) has the private field available within this internal extension, making the cast a valid approach for supporting session recreation in distributed and serverless environments.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.