-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Enhance session security with HMAC signing and rate limiting #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…on for session recreation
….json with adjusted paths
…initialization for session recreation
… Redis and Vercel KV session stores
✅ 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. |
📝 WalkthroughWalkthroughReplaces duplicated inline alpha-publish shell commands with a shared Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Registry as TransportRegistry
participant Store as SessionStore
participant RateLimiter as SessionRateLimiter
participant Crypto as SessionCrypto
Client->>Registry: getStoredSession(type, token, sessionId, {clientFingerprint})
Note right of Registry: validate fingerprint (optional)
Registry->>Store: get(sessionId, {clientIdentifier})
Note right of Store: rate-limit check
Store->>RateLimiter: check(clientIdentifier)
alt throttled
RateLimiter-->>Store: allowed=false
Store-->>Registry: return null
else allowed
RateLimiter-->>Store: allowed=true
Note right of Store: check maxLifetimeAt
alt expired
Store->>Store: delete session
Store-->>Registry: return null
else valid
Note right of Store: verify or parse payload
Store->>Crypto: verifyOrParseSession(data, config)
alt verification fails
Crypto-->>Store: null
Store->>Store: delete session
Store-->>Registry: return null
else verified
Crypto-->>Store: StoredSession
Store-->>Registry: StoredSession
end
end
end
Registry-->>Client: StoredSession | undefined
sequenceDiagram
participant Client
participant Registry as TransportRegistry
participant Crypto as SessionCrypto
participant Store as SessionStore
Client->>Registry: storeSession(type, token, sessionId, session)
Note right of Registry: signing optional
alt signing enabled
Registry->>Crypto: signSession(session, config)
Crypto-->>Registry: signedPayload
Registry->>Store: set(sessionId, signedPayload, ttl, maxLifetimeAt)
else signing disabled
Registry->>Store: set(sessionId, jsonPayload, ttl, maxLifetimeAt)
end
Store-->>Registry: persisted
Registry-->>Client: success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
libs/uipack/src/bundler/file-cache/component-builder.ts (1)
424-424: Sanitize error message to avoid exposing internal details.Directly appending the error object to a string can expose internal stack traces or sensitive implementation details. Extract the error message explicitly.
🔎 Proposed fix to sanitize error output
} catch (error) { - throw new Error(`Bundle failed for ${entryPath}: ${error}`); + const errorMessage = error instanceof Error ? error.message : String(error); + throw new Error(`Bundle failed for ${entryPath}: ${errorMessage}`); }
♻️ Duplicate comments (5)
libs/testing/project.json (1)
105-112: Same critical issues as in libs/cli/project.json.This publish-alpha target has identical issues to the one in
libs/cli/project.json:
- Unstable ngrok registry URL
- No error handling in shell script
- Code duplication across multiple files
Please refer to the review comment on
libs/cli/project.jsonlines 54-61 for detailed analysis and recommended fixes.libs/ui/project.json (1)
96-103: Same critical issues as in libs/cli/project.json.This publish-alpha target has identical issues to the ones in
libs/cli/project.jsonandlibs/testing/project.json. Please refer to the review comment onlibs/cli/project.jsonlines 54-61 for detailed analysis and recommended fixes.libs/json-schema-to-zod-v3/project.json (1)
66-73: Same critical issues as in libs/cli/project.json.This publish-alpha target has identical issues. Please refer to the review comment on
libs/cli/project.jsonlines 54-61 for detailed analysis and recommended fixes.libs/mcp-from-openapi/project.json (1)
66-73: Same critical issues as in libs/cli/project.json.This publish-alpha target has identical issues. Please refer to the review comment on
libs/cli/project.jsonlines 54-61 for detailed analysis and recommended fixes.libs/sdk/project.json (1)
95-102: Same critical issues as in libs/cli/project.json.This publish-alpha target has identical issues. Please refer to the review comment on
libs/cli/project.jsonlines 54-61 for detailed analysis and recommended fixes.
🧹 Nitpick comments (11)
libs/uipack/src/renderers/utils/transpiler.ts (1)
52-53: LGTM! Consider documenting bundler-specific behavior.The
webpackIgnoredirective correctly prevents webpack from bundling the optional@swc/coredependency at compile time, allowing it to be loaded dynamically at runtime. The implementation is sound.The comment on line 52 mentions "prevent bundler" generically, but the directive is webpack-specific. Consumers using other bundlers (esbuild, Rollup, Vite) would need to configure external dependencies through their bundler config rather than magic comments. Consider adding a note in the documentation about bundler compatibility if this library is consumed by projects using different build tools.
libs/uipack/src/bundler/file-cache/component-builder.ts (1)
105-111: ValidateexecuteCodeparameter before execution.The
executeCodecallback at line 105 is optional and passed directly torenderSSRwithout validation. If provided, ensure it conforms to the expected signature before invocation at line 467.Consider adding a type guard or validation to confirm the callback is callable before attempting to invoke it. Alternatively, document that callers must ensure the callback signature matches exactly.
Also applies to: 462-471
libs/sdk/src/auth/session/vercel-kv-session.store.ts (1)
165-175: Consider distinguishing rate-limited responses from missing sessions.Returning
nullwhen rate-limited is indistinguishable from a missing session to callers. While the warning is logged server-side, callers cannot differentiate between "session not found" and "rate limited" to respond appropriately (e.g., HTTP 429 vs 404).This may be intentional to prevent information leakage, but consider whether callers need this distinction for proper client feedback.
libs/sdk/src/transport/flows/handle.streamable-http.flow.ts (1)
323-346: SSE listener recreation follows consistent pattern.The recreation logic mirrors
onMessageappropriately. One minor inconsistency:onMessagelogsinitializedstatus from stored session (line 241), butonSseListenerdoes not (line 329). Consider adding for consistency if the initialized state is relevant for SSE listener debugging.libs/sdk/src/transport/adapters/transport.streamable-http.adapter.ts (1)
30-32: Clarify or remove commented-out code.The
// this.destroy();inonsessionclosedis unclear. If this is intentional (e.g., to avoid premature cleanup), add a comment explaining why. If it's debugging leftovers, remove it.libs/sdk/src/auth/session/session-crypto.ts (1)
185-196: Legacy session parsing lacks schema validation.When parsing unsigned legacy sessions, the code casts directly to
StoredSessionwithout validation. If the stored data is malformed or maliciously crafted, this could cause runtime errors downstream. The caller (e.g.,RedisSessionStore) does perform schema validation afterward, so this is mitigated, but documenting this assumption would improve clarity.🔎 Consider adding a note about caller validation
/** * Verify or parse a session, supporting both signed and unsigned formats. * Useful for backwards compatibility during migration. * * @param data - Raw data from storage * @param config - Optional signing configuration - * @returns The session (verified if signed, parsed if unsigned) or null + * @returns The session (verified if signed, parsed if unsigned) or null + * @note Legacy unsigned sessions are returned without validation - caller should validate with storedSessionSchema */libs/sdk/src/transport/adapters/sse-transport.ts (1)
85-88: Private field access is fragile - document SDK version dependency.Accessing
_eventIdCountervia(this as any)will break if the MCP SDK renames or restructures this internal field. Consider adding a version-pinned comment or implementing a safety check.🔎 Proposed defensive check
setEventIdCounter(eventId: number): void { // Security: Validate event ID to prevent invalid values if (!this.isValidEventId(eventId)) { console.warn( `[RecreateableSSEServerTransport] Invalid eventId: ${eventId}. ` + `Must be a non-negative integer. Ignoring.`, ); return; } - // Access the private _eventIdCounter field + // Access the private _eventIdCounter field + // WARNING: This depends on MCP SDK internals (@modelcontextprotocol/sdk ~1.25.x) + // If upgrading the SDK, verify this field still exists // eslint-disable-next-line @typescript-eslint/no-explicit-any - (this as any)._eventIdCounter = eventId; + const self = this as any; + if (!('_eventIdCounter' in self)) { + console.warn('[RecreateableSSEServerTransport] _eventIdCounter field not found - SDK may have changed'); + return; + } + self._eventIdCounter = eventId; }libs/sdk/src/auth/session/session-rate-limiter.ts (1)
196-200: Module-level singleton may cause unintended shared state.The
defaultSessionRateLimitersingleton is created at module load time. In testing or multi-tenant scenarios, this shared instance might cause unexpected rate limit interactions. Consider documenting this behavior or making it lazy-initialized.libs/sdk/src/transport/adapters/transport.sse.adapter.ts (1)
14-27: Consider extracting common transport wiring logic.Both
createTransportandcreateTransportFromSessionduplicate the error handler andonclosebinding (lines 21-25 and 49-52). Consider extracting this into a private helper method to reduce duplication and ensure consistent behavior.🔎 Proposed refactor
+ private wireTransport(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.wireTransport(transport); return transport; }Also applies to: 37-54
libs/sdk/src/transport/adapters/streamable-http-transport.ts (2)
27-32: Avoidanytype for eventStore - useunknownwith proper typing.Per coding guidelines,
anytypes should be avoided. Consider usingunknownor extracting the proper type from the MCP SDK.🔎 Proposed fix
/** * Event store for resumability support. - * Uses any to avoid complex type extraction from MCP SDK's optional options type. + * Uses unknown to maintain type safety. Cast as needed when passing to parent. */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - eventStore?: any; + eventStore?: unknown;
64-68: Add similar defensive check for isInitialized getter.The getter also accesses SDK internals. Consider adding a version comment for maintainability.
📜 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 (36)
.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/package.jsonlibs/sdk/project.jsonlibs/sdk/src/auth/session/index.tslibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/auth/session/session-crypto.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/auth/session/transport-session.manager.tslibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/auth/session/vercel-kv-session.store.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/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/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/transport/index.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 (11)
**/*.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/flows/handle.streamable-http.flow.tslibs/sdk/src/transport/transport.registry.tslibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/auth/session/session-crypto.tslibs/sdk/src/transport/index.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/transport.types.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/transport/transport.remote.tslibs/sdk/src/auth/session/transport-session.manager.tsapps/demo/src/main.tslibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/transport/adapters/sse-transport.tslibs/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/transport/__tests__/transport.registry.test.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/auth/session/index.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/transport.local.adapter.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/flows/handle.streamable-http.flow.tslibs/sdk/src/transport/transport.registry.tslibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/auth/session/session-crypto.tslibs/sdk/src/transport/index.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/transport.types.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/transport/transport.remote.tslibs/sdk/src/auth/session/transport-session.manager.tslibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/transport/adapters/sse-transport.tslibs/sdk/src/transport/__tests__/transport.registry.test.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/auth/session/index.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/transport.local.adapter.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/flows/handle.streamable-http.flow.tslibs/sdk/package.jsonlibs/ui/project.jsonlibs/sdk/project.jsonlibs/sdk/src/transport/transport.registry.tslibs/json-schema-to-zod-v3/project.jsonlibs/testing/project.jsonlibs/sdk/src/auth/session/transport-session.types.tslibs/sdk/src/auth/session/session-crypto.tslibs/sdk/src/transport/index.tslibs/plugins/project.jsonlibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/transport.types.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/transport/transport.remote.tslibs/sdk/src/auth/session/transport-session.manager.tslibs/uipack/project.jsonlibs/cli/project.jsonlibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/adapters/project.jsonlibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/transport/adapters/sse-transport.tslibs/mcp-from-openapi/project.jsonlibs/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/transport/__tests__/transport.registry.test.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/transport/transport.local.tslibs/sdk/src/auth/session/index.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/transport.local.adapter.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
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.tslibs/sdk/src/auth/session/index.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 (19)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
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/package.json.gitignorelibs/sdk/src/transport/index.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/auth/session/index.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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/package.jsonlibs/sdk/src/transport/index.tslibs/uipack/project.jsonlibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/package.json
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
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/package.jsonlibs/sdk/src/transport/index.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/common/decorators/front-mcp.decorator.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
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
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
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/sdk/project.jsonlibs/testing/project.jsonlibs/plugins/project.jsonlibs/uipack/project.jsonlibs/cli/project.jsonlibs/adapters/project.jsonlibs/mcp-from-openapi/project.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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:
.gitignorelibs/uipack/project.jsonlibs/uipack/src/renderers/utils/transpiler.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
.gitignore
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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/renderers/utils/transpiler.tslibs/sdk/src/common/decorators/front-mcp.decorator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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:
.gitignorelibs/uipack/project.json
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/*/package.json : Each library in /libs/* is independent and publishable under frontmcp/* scope
Applied to files:
.gitignorelibs/mcp-from-openapi/project.json
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
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-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/scope/flows/http.request.flow.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
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/uipack/src/renderers/utils/transpiler.tslibs/sdk/src/common/decorators/front-mcp.decorator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : Do not expose internal error details to users - sanitize error messages
Applied to files:
libs/uipack/src/renderers/utils/transpiler.tsscripts/strip-dist-from-pkg.js
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/decorators/front-mcp.decorator.ts
🧬 Code graph analysis (9)
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.registry.ts (2)
libs/sdk/src/transport/transport.types.ts (1)
TransportType(7-7)libs/sdk/src/auth/session/transport-session.types.ts (1)
StoredSession(153-172)
libs/sdk/src/auth/session/session-crypto.ts (2)
libs/sdk/src/auth/session/index.ts (6)
SignedSession(20-20)SessionSigningConfig(21-21)signSession(16-16)verifySession(17-17)isSignedSession(19-19)verifyOrParseSession(18-18)libs/sdk/src/auth/session/transport-session.types.ts (1)
StoredSession(153-172)
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/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/auth/session/session-rate-limiter.ts (1)
libs/sdk/src/auth/session/index.ts (4)
SessionRateLimiterConfig(10-10)RateLimitResult(11-11)SessionRateLimiter(9-9)defaultSessionRateLimiter(12-12)
libs/sdk/src/auth/session/redis-session.store.ts (3)
libs/sdk/src/auth/session/transport-session.types.ts (3)
RedisConfig(235-244)SessionSecurityConfig(250-290)StoredSession(153-172)libs/sdk/src/auth/session/session-rate-limiter.ts (1)
SessionRateLimiter(59-194)libs/sdk/src/auth/session/session-crypto.ts (2)
verifyOrParseSession(185-196)signSession(82-94)
libs/sdk/src/transport/transport.local.ts (1)
libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
SupportedTransport(22-26)
libs/sdk/src/transport/adapters/transport.local.adapter.ts (3)
libs/sdk/src/transport/index.ts (3)
SSEServerTransport(28-28)RecreateableStreamableHTTPServerTransport(21-21)RecreateableSSEServerTransport(25-25)libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
RecreateableStreamableHTTPServerTransport(56-89)libs/sdk/src/transport/adapters/sse-transport.ts (1)
RecreateableSSEServerTransport(34-114)
⏰ 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). (1)
- GitHub Check: Build Libraries
🔇 Additional comments (56)
.gitignore (1)
64-64: LGTM!Both additions follow best practices:
- Line 64:
/.npmrc.verdacciocorrectly prevents sensitive Verdaccio registry credentials from being committed.- Line 67: The generalized
**/tsconfig.lib.tsbuildinfopattern appropriately excludes TypeScript build cache artifacts across all packages in the monorepo.These changes align well with the publishing workflows being introduced in this PR.
Also applies to: 67-67
libs/sdk/src/common/decorators/front-mcp.decorator.ts (3)
7-10: Clear documentation of architectural decision.The updated comments effectively explain why direct relative imports are used instead of package imports, making the circular dependency concern explicit for future maintainers.
88-91: Consistent use of lazy-loading helpers.The refactoring correctly replaces direct
requirecalls with the new lazy-loading helpers, maintaining consistency with the established pattern. The updated comment clarifies the architectural decision.
23-40: Module exports are correct andunknowntype is appropriate for the handler parameter.The serverless-handler module exports exactly match the
ServerlessHandlerFnstype definition. Theunknowntype for the handler parameter is the correct choice—not a limitation. The handler comes fromServerlessInstance.createHandler(metadata)and varies by deployment (Express app, Lambda handler, etc.). Since the actual handler type is application-specific rather than SDK-specific,unknownis the appropriate generic type default per coding guidelines. The error handling correctly usesInternalMcpErrorwith the specific error code, and the lazy-loading pattern properly mirrors the existinggetFrontMcpInstance()helper.Likely an incorrect or invalid review comment.
scripts/strip-dist-from-pkg.js (6)
6-17: LGTM!Error handling is clean and appropriate for a build script. Clear error messages with exit codes facilitate debugging in CI pipelines.
19-36: LGTM!The recursive
walkfunction correctly handles the three cases (arrays, objects, primitives) with proper pruning semantics. Thedevelopmentcondition removal and empty object pruning will produce clean exports maps.
38-55: LGTM!Proper handling of top-level fields and conditional deletion of empty exports/bin maps after cleaning.
57-65: LGTM!Removing scripts from the distributed package.json is a good practice to avoid confusion and reduce package size.
89-124: LGTM!The ESM package.json generation correctly reuses the already-cleaned exports/bin from the main package processing, then applies path adjustments for the ESM subdirectory structure. Error handling is consistent with the rest of the script.
76-87: The build configuration guarantees all ESM files use.mjsextensions, making the raised concern impossible. Every library in this monorepo uses the same esbuild configuration with"outExtension": { ".js": ".mjs" }for ESM builds, ensuring no.jsfiles exist in esm directories. TheadjustForEsmfunction correctly handles the actual file structure produced by the build system.Likely an incorrect or invalid review comment.
libs/uipack/src/bundler/file-cache/component-builder.ts (7)
392-399: Clarifying comment improves maintainability.The comment explaining why esbuild is lazy-loaded due to external marking in project.json is a helpful documentation addition that prevents future confusion about the lazy-load pattern.
1-29: Imports and type definitions are well-structured.The
ReactComponentTypedefinition (lines 27–29) appropriately usesanywith a justification comment to avoid runtime React dependency while preserving type information. The justified@typescript-eslint/no-explicit-anydisable is correct per coding guidelines.
166-172: Constructor is concise and correct.Simple constructor correctly stores the storage dependency without unnecessary validation.
219-263: Dependency resolution with peer dependency traversal is robust.The resolver correctly:
- Handles resolution failures gracefully by logging warnings (line 236)
- Falls back to bundling unresolved packages instead of failing hard
- Discovers and adds peer dependencies transitively (lines 241–263)
- Continues on peer resolution errors (line 256)
401-422: esbuild transform options are well-configured.The loader selection by file extension (line 402) and transform options (lines 405–416) correctly:
- Auto-detect TypeScript vs JavaScript based on extension
- Default to ES2020 target if not specified
- Enable tree-shaking by default
- Use automatic JSX transformation with configurable import source
- Include external dependencies as a banner for later import map processing
495-519: Factory functions follow best practices with dynamic imports.Both
createFilesystemBuilderandcreateRedisBuildercorrectly:
- Use dynamic imports to avoid hard dependencies at module load time
- Properly initialize storage before returning builder
- Use type-safe imports with
import()syntax
462-471: SSR documentation should clarify that this is for trusted, build-time component code.The security warning about
new Function()is accurate, but the current phrasing may be misleading.ComponentBuilderis designed for build-time bundling of developer-owned component files (entryPath always references a local filesystem file the developer controls), not for processing untrusted user input at runtime.The code being SSR'd is the developer's bundled component code, not external user input. If someone needs to SSR untrusted code, providing a custom
executeCodehandler with proper sandboxing (e.g., Node.jsvmmodule) is the intended pattern.Consider clarifying in the JSDoc that this is for trusted component sources only, and that the
executeCodeparameter is available for developers who need stricter isolation.apps/demo/src/main.ts (1)
18-20: LGTM! Transport config correctly placed in auth object.The relocation of transport configuration under the auth object aligns with the broader changes in this PR, where the SDK now reads transport config from
scope.metadata.transportwith a fallback toscope.auth.transport.libs/sdk/src/scope/flows/http.request.flow.ts (1)
207-221: LGTM! Transport config fallback provides backward compatibility.The change to read transport config from
scope.metadata.transportwith a fallback toscope.auth.transportmaintains backward compatibility while supporting the new config structure. The added debug logging improves observability for transport decisions.Consider documenting the deprecation timeline for
auth.transportin the SDK documentation to guide users on when to migrate their configurations.libs/plugins/project.json (1)
84-90: Approve the publish-alpha target addition with a minor observation.The publish-alpha target follows the same pattern across all library project.json files in this PR. The workflow correctly:
- Depends on
buildto ensure artifacts exist- Uses
ALPHA_TSenv var for deterministic versioning across packages- Falls back to
date +%sfor local usage- Strips any existing pre-release suffix before appending
-alpha.$tsOne consideration: the command assumes
jqis available in the CI/build environment. Ensure your CI runners havejqinstalled.libs/adapters/project.json (1)
82-89: LGTM!The publish-alpha target follows the same consistent pattern established across the monorepo.
libs/uipack/project.json (1)
115-122: LGTM!Consistent publish-alpha target configuration matching the pattern across other libraries.
libs/sdk/src/transport/__tests__/transport.registry.test.ts (1)
18-18: LGTM!The mock correctly includes the new
markAsInitializedmethod to match the updatedLocalTransporterinterface. Consider adding explicit test cases that verifymarkAsInitializedis called during session recreation flows if that behavior is critical.libs/sdk/src/transport/transport.remote.ts (1)
30-33: LGTM!The no-op implementation is appropriate for
RemoteTransportersince initialization state is managed on the remote node. The comment clearly documents this design decision.libs/sdk/src/auth/session/transport-session.manager.ts (3)
30-34: Good addition of absolute session lifetime enforcement.The
maxLifetimeAtcheck prevents indefinite session extension through repeated access, which is a critical security improvement. The check correctly:
- Runs before the sliding
expiresAtcheck- Deletes the session when exceeded
- Is applied consistently in
get(),exists(), andcleanup()
62-66: Consistent maxLifetimeAt enforcement in exists().The check mirrors the
get()implementation, ensuring consistent behavior.
87-92: Cleanup correctly handles maxLifetimeAt.The
continuestatement after deleting expired sessions prevents unnecessary expiration checks on already-deleted entries.libs/sdk/src/transport/transport.local.ts (3)
6-6: LGTM!The import correctly brings in
SupportedTransportto type the adapter field.
15-15: Type update aligns with expanded transport support.Using
SupportedTransportinstead of a hardcoded union allows theLocalTransporterto work with the new recreateable transport variants (RecreateableStreamableHTTPServerTransport,RecreateableSSEServerTransport) as shown in the relevant code snippet fromtransport.local.adapter.ts.
69-76: Well-documented delegation to adapter.The
markAsInitialized()method correctly delegates to the underlying adapter, and the JSDoc clearly explains the Redis-based session recreation use case.libs/sdk/src/transport/transport.types.ts (1)
63-69: Interface change is well-documented and all implementations updated.The JSDoc clearly explains the purpose of
markAsInitialized()for Redis-based session recreation. Both internal implementations are properly updated:
RemoteTransporterdelegates to the adapter'smarkAsInitialized()methodLocalTransporterimplements it as a no-op with a clear comment noting that initialization state is managed on the remote nodeNo external
Transporterimplementations exist in the repository that need updating.libs/sdk/package.json (1)
55-65: LGTM! New transport export entry is well-structured.The new
./transportexport follows the established pattern for dual CJS/ESM support with development source access. This properly exposes the new recreateable transport classes through@frontmcp/sdk/transport.libs/sdk/src/auth/session/vercel-kv-session.store.ts (4)
78-97: LGTM! Security configuration initialization is correct.The security configuration is properly defaulted to an empty object, and the rate limiter is conditionally instantiated only when enabled. The optional chaining on
config.securityhandles bothVercelKvSessionConfigandVercelKvProviderOptionscorrectly.
194-210: Signature verification logic handles both signed and unsigned sessions correctly.The code properly handles the dual path where:
- Signed sessions are verified and extracted via
verifyOrParseSession- Unsigned sessions are parsed directly with appropriate type handling for Vercel KV's auto-parsing behavior
The fallback deletion of sessions with failed verification (line 205) is appropriate for security.
226-234: Max lifetime enforcement is correctly implemented.The absolute maximum lifetime check prevents indefinite session extension via sliding expiration, which is a critical security feature. The synchronous deletion before returning null ensures proper cleanup.
281-288: LGTM! HMAC signing in set path is correctly implemented.The conditional signing with fallback to plain JSON serialization is clean and follows the established pattern. The signing secret is properly sourced from the security configuration.
libs/sdk/src/transport/adapters/transport.local.adapter.ts (2)
16-26: SupportedTransport union is well-documented but somewhat redundant.The JSDoc correctly notes that
RecreateableStreamableHTTPServerTransportextendsStreamableHTTPServerTransport(and similarly for SSE). Since the Recreateable variants extend the base types, they're already assignable to the base type in the union. However, explicitly listing all four types improves discoverability and self-documentation.
61-67: LGTM! markAsInitialized hook enables session recreation.The no-op default with clear JSDoc guidance for subclass overrides follows the template method pattern appropriately. This enables the transport registry to mark recreated transports as initialized without coupling to specific transport implementations.
libs/sdk/src/transport/transport.registry.ts (3)
209-222: Fingerprint validation logic is secure with correct default behavior.The implementation correctly:
- Only validates when both request fingerprint and stored fingerprint exist
- Defaults to rejecting mismatches (secure default)
- Allows opt-in warning-only mode via
warnOnFingerprintMismatch- Logs truncated fingerprints (8 chars) to aid debugging without full exposure
318-324: LGTM! Initialization state restoration with backwards compatibility.The condition
storedSession.initialized !== falsecorrectly handles:
true→ marks as initializedundefined→ marks as initialized (backwards compat for old sessions)false→ skips marking (session wasn't fully initialized)This ensures recreated transports can accept requests without re-initialization handshake.
414-427: Consider when to setinitialized: truefor accurate session state.The
initialized: trueflag is set immediately when persisting the session duringcreateTransporter. However, the MCP protocol initialization handshake may not be complete at this point (it happens intransport.initialize()which is called later inonInitialize).If the server crashes between
createTransporterand the completion oftransport.initialize(), the session will be marked as initialized but may not have completed the handshake.Consider whether
initializedshould be set tofalseinitially and updated totrueafter theinitializerequest completes, or document this as acceptable given the short window.libs/sdk/src/transport/flows/handle.streamable-http.flow.ts (1)
217-250: Observability improvements are well-structured.The additional logging provides valuable insight into the transport lifecycle:
- Session IDs are properly truncated (first 20 chars)
- Log levels are appropriate:
infofor normal flow,warnfor failures- Logs include relevant context (
initialized,createdAt) without sensitive dataFor high-traffic production environments, consider whether
infolevel logs for every message might be excessive, or ensure log sampling/filtering is configured.libs/sdk/src/transport/index.ts (1)
1-31: LGTM! Well-structured barrel file for transport module.The barrel file follows project conventions with:
- Clear JSDoc documentation including usage example
- Logical grouping: recreateable transports, legacy compatibility, types
- Consistent with the coding guideline to export users' needed items through barrel files
libs/sdk/src/auth/session/transport-session.types.ts (2)
246-290: SessionSecurityConfig is well-designed with sensible defaults.The security configuration interface provides:
maxLifetimeMs: Prevents indefinite session extension (security best practice)enableSigning/signingSecret: HMAC protection with env var fallback- Rate limiting configuration with reasonable defaults (60s window, 100 requests)
The JSDoc is thorough with examples and default values documented.
164-171: LGTM! StoredSession extensions with backwards compatibility.The new
initializedandmaxLifetimeAtfields are correctly:
- Optional in both the TypeScript interface and Zod schema
- Well-documented with JSDoc explaining their purpose
- Compatible with existing stored sessions (no migration required)
Also applies to: 387-388
libs/sdk/src/auth/session/index.ts (1)
4-22: LGTM! Barrel file properly exposes new session security APIs.The updated exports correctly expose:
- Store configurations alongside store classes for consumer convenience
- Rate limiting utilities with both class and default singleton
- Crypto utilities for signing/verification with all related types
This follows the coding guideline to export users' needed items through barrel files.
libs/sdk/src/transport/adapters/transport.streamable-http.adapter.ts (1)
141-147: LGTM - Correct use of public API for session recreation.The
markAsInitializedmethod properly uses the publicsetInitializationStateAPI instead of accessing private properties, and includes appropriate logging. Minor note: the sessionId slice length (20) differs from other logging in this file (30 at line 52), but this is a cosmetic inconsistency.libs/sdk/src/auth/session/session-crypto.ts (2)
41-57: LGTM - Secure secret handling with appropriate production safeguards.The implementation correctly throws in production when no secret is configured and provides a clear warning in development. The warning message exposes the fallback value which is acceptable since it's only for local development.
82-94: Verify JSON serialization determinism across sign and verify.The signature is computed over
JSON.stringify(session), and verification re-serializessigned.data(line 137). If the session object contains properties in a different order after JSON round-trip, the signature could fail. JavaScript'sJSON.stringifypreserves insertion order for own properties, so this should work correctly, but be aware of edge cases with prototype chains or symbols.libs/sdk/src/transport/adapters/sse-transport.ts (1)
97-113: Session ID mismatch should log more context for debugging.When the session ID doesn't match, the warning is logged but the code continues. This is reasonable for recreation scenarios, but logging both IDs fully (not truncated) could aid debugging in non-production environments.
libs/sdk/src/auth/session/session-rate-limiter.ts (2)
59-76: LGTM - Clean rate limiter implementation.The sliding window algorithm is correctly implemented with proper cleanup handling. Using
unref()on the timer ensures the process can exit cleanly. The default values (100 requests per minute) are reasonable.
1-8: Rate limiting protection against session enumeration is ineffective as implemented.The documentation states the rate limiter protects against "session enumeration attacks" using per-IP limiting, and the example shows
rateLimiter.check(clientIp). However, bothRedisSessionStore.get()andVercelKvSessionStore.get()callrateLimiter.check(sessionId)instead. Per-sessionId rate limiting allows attackers to bypass enumeration protection by trying different sessionIds—each gets its own quota. The SessionStore interface receives onlysessionId, not client IP, making per-IP limiting impossible at this layer. Either update the documentation to clarify this limitation, or refactor to pass client IP context through the store interface to implement the claimed protection.libs/sdk/src/transport/adapters/transport.sse.adapter.ts (1)
29-54: LGTM - Well-documented session recreation method.The
createTransportFromSessionmethod is properly documented and correctly passesinitialEventIdto support SSE reconnection. The logging includes useful context for debugging.libs/sdk/src/auth/session/redis-session.store.ts (3)
140-151: LGTM - Proper signature verification with secure deletion.The code correctly verifies signed sessions and deletes tampered sessions. The fire-and-forget pattern with
.catch(() => void 0)is acceptable here since we're already returning null.
167-175: LGTM - Correct max lifetime enforcement.The absolute maximum lifetime check properly prevents indefinite session extension. Using
await this.delete()here is correct since we want to ensure cleanup before returning.
227-234: LGTM - Signing correctly applied during set.The HMAC signing is properly applied when
enableSigningis configured, with the secret passed through appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libs/sdk/src/auth/session/redis-session.store.ts (1)
113-127: Past review comment properly addressed.The addition of
clientIdentifiersupport (line 117) correctly addresses the previous concern about session enumeration protection. The implementation now rate-limits by client IP when available, falling back to sessionId for per-session DoS protection.
🧹 Nitpick comments (6)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (3)
85-88: Good validation logic addresses the previous review.The sessionId validation correctly handles empty, non-string, and whitespace-only values using short-circuit evaluation to avoid TypeErrors. The fail-fast approach (throwing an error) is appropriate for session security.
For enhanced session security, consider adding format validation to ensure sessionIds meet expected patterns (e.g., minimum length, allowed characters). This would provide defense-in-depth against malformed or malicious session identifiers.
🔎 Optional: Add sessionId format validation
setInitializationState(sessionId: string): void { // Validate sessionId if (!sessionId || typeof sessionId !== 'string' || sessionId.trim() === '') { throw new Error('[RecreateableStreamableHTTPServerTransport] sessionId cannot be empty'); } + // Optional: Validate format for additional security + if (sessionId.length < 16 || !/^[a-zA-Z0-9_-]+$/.test(sessionId)) { + throw new Error('[RecreateableStreamableHTTPServerTransport] sessionId format is invalid'); + }
94-100: Good defensive check, but consider validating internal field existence too.The webTransport existence check properly handles SDK version incompatibilities by logging a warning and returning early.
The previous review also suggested checking if the
_initializedfield exists before setting it. This would provide additional safety against SDK changes that rename or remove this internal field, preventing silent failures where properties are created but not recognized by the SDK.🔎 Recommended: Add field existence validation
const webTransport = (this as any)._webStandardTransport; if (!webTransport) { console.warn( '[RecreateableStreamableHTTPServerTransport] Internal transport not found. ' + 'This may indicate an incompatible MCP SDK version.', ); return; } + if (!('_initialized' in webTransport) || !('sessionId' in webTransport)) { + console.warn( + '[RecreateableStreamableHTTPServerTransport] Internal fields not found. ' + + 'This may indicate an incompatible MCP SDK version.', + ); + return; + }
102-103: Consider logging when overwriting existing session state.The code correctly sets the internal state after validation. However, if
setInitializationStateis called multiple times with different sessionIds, the change occurs silently.For session security and debugging, consider logging when an existing sessionId is being changed, as this could help detect unexpected session state transitions.
🔎 Optional: Add logging for session state changes
+ // Log if we're changing an existing sessionId + if (webTransport._initialized && webTransport.sessionId && webTransport.sessionId !== sessionId) { + console.warn( + `[RecreateableStreamableHTTPServerTransport] Changing sessionId from ${webTransport.sessionId} to ${sessionId}`, + ); + } + webTransport._initialized = true; webTransport.sessionId = sessionId;scripts/publish-alpha.sh (2)
29-36: Consider adding validation for environment variables.While the defaults are sensible, the script doesn't validate that
VERDACCIO_REGISTRYis a valid URL or thatALPHA_TSis numeric. This could lead to cryptic npm errors if these are misconfigured.🔎 Optional: Add basic validation
# Use environment variable or default REGISTRY="${VERDACCIO_REGISTRY:-https://verdaccio.ngrok.app}" +if [[ ! "$REGISTRY" =~ ^https?:// ]]; then + echo "Error: VERDACCIO_REGISTRY must be a valid HTTP(S) URL" + exit 1 +fi + TIMESTAMP="${ALPHA_TS:-$(date +%s)}" +if [[ ! "$TIMESTAMP" =~ ^[0-9]+$ ]]; then + echo "Error: ALPHA_TS must be a numeric timestamp" + exit 1 +fi
38-56: LGTM! Version manipulation and publishing logic is sound.The script correctly:
- Extracts the base version by stripping prerelease suffixes
- Uses
jq --argfor safe variable interpolation- Validates the extracted version
- Publishes with appropriate npm flags
Minor enhancement: Consider making the temporary file handling more explicit with cleanup:
🔎 Optional: Add explicit temp file cleanup
# Update version in package.json ALPHA_VERSION="$BASE_VERSION-alpha.$TIMESTAMP" -jq --arg v "$ALPHA_VERSION" '.version=$v' "$PROJECT_DIST/package.json" > "$PROJECT_DIST/package.tmp.json" -mv "$PROJECT_DIST/package.tmp.json" "$PROJECT_DIST/package.json" +TEMP_FILE="$PROJECT_DIST/package.tmp.json" +jq --arg v "$ALPHA_VERSION" '.version=$v' "$PROJECT_DIST/package.json" > "$TEMP_FILE" +mv "$TEMP_FILE" "$PROJECT_DIST/package.json" || { rm -f "$TEMP_FILE"; exit 1; }This ensures the temp file is cleaned up if
mvfails (though withset -ethe script would exit anyway).libs/sdk/src/auth/session/vercel-kv-session.store.ts (1)
206-207: Consider the necessity of JSON.stringify fallback.The conversion
typeof raw === 'string' ? raw : JSON.stringify(raw)handles cases where Vercel KV might return a parsed object. However, if you're always storing strings (lines 294-296), this fallback may be unnecessary. Verify whether the@vercel/kvclient automatically deserializes JSON, which would explain this pattern.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
libs/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/redis-session.store.tslibs/sdk/src/auth/session/session-crypto.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/sdk/src/transport/adapters/sse-transport.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.sse.adapter.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/testing/project.jsonlibs/ui/project.jsonlibs/uipack/project.jsonlibs/uipack/src/bundler/file-cache/component-builder.tslibs/uipack/src/renderers/utils/transpiler.tsscripts/publish-alpha.sh
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/cli/project.json
- libs/sdk/src/transport/adapters/sse-transport.ts
- libs/sdk/src/transport/adapters/transport.sse.adapter.ts
- libs/plugins/project.json
- libs/mcp-from-openapi/project.json
- libs/uipack/project.json
- libs/sdk/src/transport/adapters/transport.streamable-http.adapter.ts
- libs/uipack/src/renderers/utils/transpiler.ts
🧰 Additional context used
📓 Path-based instructions (6)
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/json-schema-to-zod-v3/project.jsonlibs/sdk/src/auth/session/session-crypto.tslibs/ui/project.jsonlibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/sdk/project.jsonlibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/uipack/src/bundler/file-cache/component-builder.tslibs/testing/project.jsonlibs/adapters/project.json
**/*.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/auth/session/session-crypto.tslibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/uipack/src/bundler/file-cache/component-builder.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/auth/session/session-crypto.tslibs/sdk/src/auth/session/redis-session.store.tslibs/sdk/src/auth/session/vercel-kv-session.store.tslibs/sdk/src/auth/session/session-rate-limiter.tslibs/sdk/src/transport/adapters/streamable-http-transport.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.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.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
🧠 Learnings (6)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Use Nx for build system with commands like `nx build sdk`, `nx test`, `nx run-many -t test`
Applied to files:
libs/json-schema-to-zod-v3/project.jsonlibs/ui/project.jsonlibs/sdk/project.jsonlibs/testing/project.jsonlibs/adapters/project.json
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/auth/session/session-crypto.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : Do not expose internal error details to users - sanitize error messages
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
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.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
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:
libs/uipack/src/bundler/file-cache/component-builder.ts
🧬 Code graph analysis (3)
libs/sdk/src/auth/session/session-crypto.ts (1)
libs/sdk/src/auth/session/transport-session.types.ts (1)
StoredSession(153-172)
libs/sdk/src/auth/session/vercel-kv-session.store.ts (4)
libs/sdk/src/auth/session/transport-session.types.ts (2)
SessionSecurityConfig(250-290)StoredSession(153-172)libs/sdk/src/auth/session/session-rate-limiter.ts (1)
SessionRateLimiter(59-194)libs/sdk/src/auth/session/redis-session.store.ts (1)
key(94-99)libs/sdk/src/auth/session/session-crypto.ts (2)
verifyOrParseSession(185-198)signSession(82-94)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
libs/testing/src/client/mcp-test-client.ts (1)
sessionId(207-209)
⏰ 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). (2)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
🔇 Additional comments (24)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (4)
29-31: LGTM! Good documentation of theanytype usage.The expanded comment properly justifies the use of
anyby explaining that the EventStore interface is not exported and varies between SDK versions. This aligns with the coding guideline requiring strong justification foranytypes.
67-70: LGTM! Improved documentation for SDK internal access.The updated comment appropriately documents the SDK version concerns and use of optional chaining for defensive access to private properties.
82-82: LGTM! Good API documentation.Adding the
@throwsannotation properly documents the error condition for API consumers.
91-91: LGTM! Enhanced documentation of SDK internal dependency.The comment update appropriately highlights the SDK version compatibility risk.
libs/ui/project.json (1)
96-103: LGTM! Consistent with centralized publishing approach.The change aligns with the repository-wide refactor to use the shared
publish-alpha.shscript.libs/adapters/project.json (1)
82-89: LGTM! Consistent with centralized publishing approach.The change aligns with the repository-wide refactor to use the shared
publish-alpha.shscript.libs/testing/project.json (1)
105-112: LGTM! Consistent with centralized publishing approach.The change aligns with the repository-wide refactor to use the shared
publish-alpha.shscript.scripts/publish-alpha.sh (2)
1-9: LGTM! Proper shell script setup.The script uses appropriate bash best practices with
set -euo pipefailfor strict error handling.
11-27: LGTM! Comprehensive input validation.The validation logic properly checks for missing arguments, directory existence, and required files with clear error messages.
libs/json-schema-to-zod-v3/project.json (1)
66-73: LGTM! Consistent with centralized publishing approach.The change aligns with the repository-wide refactor to use the shared
publish-alpha.shscript. Note that this library uses independent versioning, but the script correctly handles this by reading the version from each package'spackage.json.libs/sdk/project.json (1)
95-102: LGTM! Centralized alpha publishing logic is correctly implemented.The refactor to use an external script improves maintainability by centralizing the alpha publishing workflow across all libraries. The script is properly implemented with error handling, configurable registry support, and correct version management. The configuration is consistent across all 9 libraries (adapters, cli, json-schema-to-zod-v3, mcp-from-openapi, plugins, sdk, testing, ui, uipack).
libs/sdk/src/auth/session/vercel-kv-session.store.ts (4)
7-11: Excellent documentation of atomicity limitations.The warning clearly explains the trade-offs of using Vercel KV versus Redis for session storage. This helps users make informed decisions based on their atomicity requirements.
88-97: Clean security feature initialization.The initialization properly handles optional security configuration and conditionally creates the rate limiter only when needed.
170-184: Well-designed rate limiting with proper fallback strategy.The implementation correctly uses
clientIdentifierfor enumeration protection and falls back tosessionIdfor per-session DoS protection. The logging approach with truncated identifiers balances observability with privacy.
235-243: Proper enforcement of absolute session lifetime.The implementation correctly prevents indefinite session extension by checking
maxLifetimeAtand proactively deleting expired sessions. The use ofawaitensures the deletion completes before returning, preventing race conditions.libs/sdk/src/auth/session/session-crypto.ts (4)
41-57: Secure secret management with appropriate dev/prod handling.The implementation correctly requires secrets in production while allowing a development default with a clear warning. This pattern aligns with the codebase's approach to development convenience without compromising production security.
Based on learnings, development-only modes are intentionally supported for local convenience.
140-152: Excellent use of timing-safe comparison.The implementation correctly uses
timingSafeEqualto prevent timing attacks during signature verification. The length check before comparison (lines 144-147) is necessary for the timing-safe comparison to work properly.
82-94: Clean signing implementation with versioning support.The versioned signature format (v: 1) provides a clear upgrade path for future cryptographic algorithm changes, which is excellent forward-thinking design.
185-198: Function is a public API export—reconsider the validation contract.The function is correctly exported from
index.ts(line 18) as part of the public SDK API. While both internal callers (redis-session.store.ts:162andvercel-kv-session.store.ts:221) properly validate the result withstoredSessionSchema.safeParse(), the documentation comment only referencesRedisSessionStore.get(). External SDK consumers using this public export may not follow the undocumented validation requirement. Either move validation into the function itself or explicitly document the contract in the function's JSDoc.libs/sdk/src/auth/session/session-rate-limiter.ts (3)
84-121: Correct sliding window rate limiting implementation.The algorithm properly maintains a sliding window of timestamps and accurately calculates the reset time based on the oldest entry. The
resetAtcalculation (line 100) correctly determines when the rate limit will reset as the oldest request expires from the window.
72-74: Proper cleanup timer management with unref().Using
unref()on the cleanup timer (line 74) is excellent practice. It prevents the timer from blocking process exit while still allowing cleanup to run when the process is active.
196-209: Singleton usage properly documented.The warning about shared state and the recommendation to create per-store instances is clear and appropriate. Users are well-informed about the trade-offs.
libs/sdk/src/auth/session/redis-session.store.ts (2)
15-21: Consistent security configuration across session stores.The
RedisSessionStoreConfiginterface properly extends the base configuration with security options, maintaining API consistency withVercelKvSessionConfig.
146-160: Appropriate signature verification for Redis client.The implementation correctly uses
rawstring directly without conversion, which is appropriate since the ioredis client consistently returns strings for string values, unlike the Vercel KV client which may deserialize JSON.
…r messages in component builder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/uipack/src/bundler/file-cache/component-builder.ts (1)
196-196: Sanitize absolute path in error message for consistency.This error message exposes the absolute file path, which is inconsistent with the sanitization approach implemented at lines 424-427. Consider using a relative path (relative to
process.cwd()) or just the filename to avoid exposing internal directory structure.🔎 Proposed fix
+ const displayPath = basename(absoluteEntryPath); if (!existsSync(absoluteEntryPath)) { - throw new Error(`Entry file not found: ${absoluteEntryPath}`); + throw new Error(`Entry file not found: ${displayPath}`); }Based on learnings: Do not expose internal error details to users - sanitize error messages.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/sdk/src/transport/adapters/streamable-http-transport.tslibs/uipack/src/bundler/file-cache/component-builder.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/adapters/streamable-http-transport.tslibs/uipack/src/bundler/file-cache/component-builder.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/adapters/streamable-http-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/adapters/streamable-http-transport.tslibs/uipack/src/bundler/file-cache/component-builder.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.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.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
🧠 Learnings (5)
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : Do not expose internal error details to users - sanitize error messages
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
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.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to **/*.test.{ts,tsx} : Include error class `instanceof` checks in tests to validate error handling
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/uipack/src/bundler/file-cache/component-builder.ts
🧬 Code graph analysis (1)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (1)
libs/testing/src/client/mcp-test-client.ts (1)
sessionId(207-209)
⏰ 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). (2)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
🔇 Additional comments (3)
libs/sdk/src/transport/adapters/streamable-http-transport.ts (3)
29-34: Good justification foranytype.The comment properly documents why
anyis necessary here - theEventStoreinterface is internal to the MCP SDK and not exported. This satisfies the coding guideline requiring strong justification foranyusage.
66-71: Well-implemented defensive getter.The use of optional chaining with a
falsefallback ensures graceful degradation if the MCP SDK internals change. The comment appropriately warns about version variability.
84-113: Defensive validation properly implemented.The method now:
- Validates
sessionIdand throws immediately for invalid input (lines 86-88)- Guards against missing internal transport with appropriate warning (lines 94-100)
- Verifies expected fields exist before mutation (lines 103-109)
This addresses the concerns from the previous review. The use of
console.warnwith early return for SDK incompatibility (rather than throwing) is appropriate since these conditions indicate environment/version issues rather than caller errors.
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.