fix: moving zod to peerDependencies, adding E2E test resilience, and enhancing test client with clientInfo support. #131
Conversation
… project configuration
WalkthroughAdds a universal UI renderer and runtime, static-HTML bundler, TypeScript typings fetcher with LRU type cache, platform-aware test-client and meta plumbing, centralized machine-id and session platform detection, removes HTMX from UI components, updates flows to structuredContent, expands tests/fixtures, and adjusts CI and build configs. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant StreamTransport
participant SessionFlow
participant AuthService
participant MachineID
participant Server
Browser->>StreamTransport: request (includes User-Agent)
StreamTransport->>SessionFlow: forward input (userAgent)
SessionFlow->>AuthService: detectPlatformFromUserAgent(userAgent)
AuthService-->>SessionFlow: platformType
SessionFlow->>MachineID: getMachineId()
MachineID-->>SessionFlow: machineId
SessionFlow->>Server: build/verify session payload (platformType, nodeId)
Server-->>Browser: session response (platform-aware)
sequenceDiagram
participant ToolFlow
participant MetaBuilder
participant Detector
participant RendererRegistry
participant Renderer
participant Client
ToolFlow->>MetaBuilder: request platform meta (platformType, capabilities)
MetaBuilder-->>ToolFlow: platform-scoped meta keys
ToolFlow->>Detector: supply tool output
Detector-->>RendererRegistry: ask detect(content)
RendererRegistry->>Renderer: select renderer
Renderer->>Client: render UI (or return failure)
ToolFlow->>Client: deliver structuredContent + platform meta
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Areas to focus during review:
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 (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)docs/draft/docs/**⚙️ CodeRabbit configuration file
Files:
docs/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📓 Common learnings⏰ 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)
🔇 Additional comments (5)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/ui/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/ui/package.json
🔇 Additional comments (2)
libs/ui/package.json (2)
71-74: Appropriate use of optionalDependencies.Moving
@swc/coretooptionalDependenciesis correct. This allows the library to function without SWC when not needed, while making it available for advanced build scenarios. Similarly, keeping@mdx-js/mdxas optional provides MDX support only when required.
69-69: No action required. The library code is fully compatible with Zod 4.0.0.All component schemas use patterns that are compatible with both Zod 3 and 4, with no usage of breaking change APIs. The codebase already uses the Zod 4 format for
z.record()with two arguments, indicating the code was designed for Zod 4 compatibility. No deprecated patterns (.email(),.uuid(),.messageparameter,.default(),z.function()) are present in the schema definitions.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/testing/src/transport/transport.interface.ts (1)
6-6: Consider relocating ClientInfo type definition to avoid layering concerns.The transport layer now imports
ClientInfofrom the client types module, creating a dependency from lower-level (transport) to higher-level (client) code. While this is a type-only import and doesn't create runtime circular dependencies, it couples the layers.Consider moving the
ClientInfotype definition to either:
- A shared types module (e.g.,
libs/testing/src/types/shared.types.ts)- The transport layer itself (if it's primarily transport-related metadata)
This would maintain clearer separation of concerns and make the transport layer more independently reusable.
Also applies to: 149-150
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/testing/src/client/mcp-test-client.ts(1 hunks)libs/testing/src/client/mcp-test-client.types.ts(3 hunks)libs/testing/src/transport/streamable-http.transport.ts(4 hunks)libs/testing/src/transport/transport.interface.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/testing/src/transport/transport.interface.tslibs/testing/src/client/mcp-test-client.types.tslibs/testing/src/transport/streamable-http.transport.tslibs/testing/src/client/mcp-test-client.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/testing/src/transport/transport.interface.tslibs/testing/src/client/mcp-test-client.types.tslibs/testing/src/transport/streamable-http.transport.tslibs/testing/src/client/mcp-test-client.ts
🧠 Learnings (3)
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/testing/src/transport/transport.interface.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/testing/src/transport/transport.interface.tslibs/testing/src/client/mcp-test-client.types.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/testing/src/transport/transport.interface.tslibs/testing/src/client/mcp-test-client.types.ts
🧬 Code graph analysis (1)
libs/testing/src/transport/transport.interface.ts (1)
libs/testing/src/client/mcp-test-client.types.ts (1)
ClientInfo(76-76)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (5)
libs/testing/src/client/mcp-test-client.ts (1)
805-805: LGTM!The clientInfo is correctly forwarded from the config to the transport constructor, consistent with how other configuration properties are passed through.
libs/testing/src/client/mcp-test-client.types.ts (2)
101-102: Good type refinement from inline shape to typed alias.The change from an inline type
{ name: string; version: string }to theClientInfotype alias improves type safety and aligns with the MCP SDK'sImplementationtype, which may include additional optional metadata fields.
69-76: Correct the MCP SDK version reference.The review comment references
@modelcontextprotocol/sdk v1.24.3, but this version does not exist. The latest version is 1.18.1.Regarding the concern about
Implementationtype properties: based on consistent usage across all MCP SDK examples, bothnameandversionare provided as required properties when constructing servers and clients, and the code instreamable-http.transport.tsaccesses them without null checks, indicating they are expected to be non-optional.Likely an incorrect or invalid review comment.
libs/testing/src/transport/streamable-http.transport.ts (2)
14-14: LGTM! Type definitions are correct.The import and config type updates correctly incorporate the optional
clientInfofield, maintaining type safety and consistency with the transport interface definition.Also applies to: 25-28, 46-46
418-422: Verify property access is safe or add defensive checks.The User-Agent header generation accesses
clientInfo.nameandclientInfo.versiondirectly:headers['User-Agent'] = `${this.config.clientInfo.name}/${this.config.clientInfo.version}`;If the
Implementationtype from the MCP SDK hasnameorversionas optional properties, this could result inundefinedbeing interpolated into the header string (e.g.,undefined/undefined).Based on coding guidelines (avoid non-null assertions, use proper error handling), consider adding defensive checks:
if (this.config.clientInfo) { - headers['User-Agent'] = `${this.config.clientInfo.name}/${this.config.clientInfo.version}`; + const name = this.config.clientInfo.name ?? 'unknown'; + const version = this.config.clientInfo.version ?? '0.0.0'; + headers['User-Agent'] = `${name}/${version}`; }Alternatively, if
nameandversionare guaranteed to be required properties in theImplementationtype, this is fine as-is, but the verification from the previous comment should confirm this.As per coding guidelines, avoid assumptions about type properties without defensive checks.
frontegg-david
changed the title
frontegg-david
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/sdk/src/auth/flows/session.verify.flow.ts (1)
112-112: Optional: Remove redundant nullish coalescing.The
?? undefinedis redundant since the expression already evaluates tostring | undefined. While harmless, removing it would improve clarity.Apply this diff:
- const userAgent = (request.headers?.['user-agent'] as string | undefined) ?? undefined; + const userAgent = (request.headers?.['user-agent'] as string | undefined);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/sdk/src/auth/flows/session.verify.flow.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/auth/flows/session.verify.flow.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/auth/flows/session.verify.flow.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/auth/flows/session.verify.flow.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/auth/flows/session.verify.flow.ts
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (5)
libs/sdk/src/auth/flows/session.verify.flow.ts (5)
36-36: LGTM!The userAgent field is properly typed as an optional string using correct Zod 4 syntax, which is appropriate since the User-Agent header may not always be present in requests.
117-126: LGTM!The state management correctly uses
state.set()to propagate the userAgent through the flow state, following the established pattern and coding guidelines.
26-26: The import and usage ofdetectPlatformFromUserAgentare correctly implemented. The function signature matches its usage, returningAIPlatformTypewhich safely handles the 'unknown' value. State management properly usesstate.set()to store the User-Agent, and the conditional spread operator correctly checksplatformType !== 'unknown'before including it in the payload. No issues found.
195-202: [Your rewritten review comment text here]
[Exactly ONE classification tag]
189-191: Code is type-safe and correctly handles undefined values.The implementation properly addresses the original concerns:
detectPlatformFromUserAgentsafely handlesundefineduserAgent by returning'unknown'(line 182 of notification.service.ts)platformDetectionConfigtype matches the function parameter (optionalPlatformDetectionConfig)- Return type
AIPlatformTypeincludes the'unknown'literal, making the!== 'unknown'comparison at line 201 valid- Optional chaining for
this.scope.metadata.transport?.platformDetectionis appropriateNo changes needed.
Add [PLATFORM_DEBUG] console.log statements at 5 key points to trace platform detection flow and identify why tests fail in CI (Linux) but pass locally (Mac). Debug points: 1. Client - User-Agent header being sent 2. Server - User-Agent header received in parseInput 3. Server - Platform detection in handlePublicMode 4. Server - applyUI platform type resolution 5. Client - Response _meta check for hasToolUI
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
389-410: Replace console.log with this.logger.verbose() for consistency.The
applyUImethod already usesthis.logger.verbose()extensively for tracing (lines 368, 373, 415, etc.), but these new debug statements useconsole.loginstead. This bypasses the logging infrastructure and will execute unconditionally regardless of configured log levels.Apply this diff to use the existing logger:
- console.log( - `[PLATFORM_DEBUG] applyUI - sessionIdPayload.platformType: ${authInfo?.sessionIdPayload?.platformType}`, - ); - console.log( - `[PLATFORM_DEBUG] applyUI - notifications.getPlatformType: ${ - sessionId ? scope.notifications.getPlatformType(sessionId) : 'no-session' - }`, - ); const platformType = authInfo?.sessionIdPayload?.platformType ?? (sessionId ? scope.notifications.getPlatformType(sessionId) : undefined) ?? 'unknown'; - console.log(`[PLATFORM_DEBUG] applyUI - final platformType: ${platformType}`); + this.logger.verbose('applyUI: platform detection', { + sessionIdPayloadPlatform: authInfo?.sessionIdPayload?.platformType, + notificationsPlatform: sessionId ? scope.notifications.getPlatformType(sessionId) : 'no-session', + finalPlatform: platformType, + }); // Resolve the effective serving mode based on configuration and client capabilities // Default is 'auto' which selects the best mode for the platform const configuredMode = tool.metadata.ui.servingMode ?? 'auto'; const resolvedMode = resolveServingMode({ configuredMode, platformType, }); - console.log(`[PLATFORM_DEBUG] applyUI - supportsUI: ${resolvedMode.supportsUI}, reason: ${resolvedMode.reason}`); + this.logger.verbose('applyUI: resolved serving mode', { + configuredMode, + supportsUI: resolvedMode.supportsUI, + effectiveMode: resolvedMode.effectiveMode, + reason: resolvedMode.reason, + });
🧹 Nitpick comments (1)
libs/sdk/src/notification/notification.service.ts (1)
181-203: Consider making debug logs conditional or removing them before production.The
[PLATFORM_DEBUG]console.log statements will execute unconditionally in production. Since this is a module-level function without logger access, consider either:
- Making them conditional on an environment variable (e.g.,
process.env['DEBUG_PLATFORM'])- Removing them if they were only needed for development debugging
- Accepting a logger parameter if this tracing is needed long-term
Apply this pattern to make debug logs conditional:
export function detectPlatformFromUserAgent(userAgent?: string, config?: PlatformDetectionConfig): AIPlatformType { - console.log(`[PLATFORM_DEBUG] detectPlatformFromUserAgent input: "${userAgent}"`); + if (process.env['DEBUG_PLATFORM']) { + console.log(`[PLATFORM_DEBUG] detectPlatformFromUserAgent input: "${userAgent}"`); + } if (!userAgent) { - console.log(`[PLATFORM_DEBUG] No userAgent - returning 'unknown'`); + if (process.env['DEBUG_PLATFORM']) { + console.log(`[PLATFORM_DEBUG] No userAgent - returning 'unknown'`); + } return 'unknown'; } // Check custom mappings first const customMatch = matchCustomMappings(userAgent, config?.mappings); if (customMatch) { return customMatch; } // If customOnly, don't use default detection if (config?.customOnly) { return 'unknown'; } // Use default detection on user-agent const result = defaultPlatformDetection(userAgent); - console.log(`[PLATFORM_DEBUG] defaultPlatformDetection result: ${result}`); + if (process.env['DEBUG_PLATFORM']) { + console.log(`[PLATFORM_DEBUG] defaultPlatformDetection result: ${result}`); + } return result; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CLAUDE.md(1 hunks)libs/sdk/src/auth/flows/session.verify.flow.ts(5 hunks)libs/sdk/src/notification/notification.service.ts(2 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(2 hunks)libs/testing/src/client/mcp-test-client.ts(2 hunks)libs/testing/src/transport/streamable-http.transport.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/sdk/src/auth/flows/session.verify.flow.ts
- libs/testing/src/client/mcp-test-client.ts
- libs/testing/src/transport/streamable-http.transport.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/notification/notification.service.tslibs/sdk/src/tool/flows/call-tool.flow.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/notification/notification.service.tslibs/sdk/src/tool/flows/call-tool.flow.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/notification/notification.service.tslibs/sdk/src/tool/flows/call-tool.flow.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
🧬 Code graph analysis (1)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
libs/testing/src/client/mcp-test-client.ts (1)
sessionId(202-204)
Add [PLATFORM_DEBUG] console.log statements at 5 key points to trace platform detection flow and identify why tests fail in CI (Linux) but pass locally (Mac). Debug points: 1. Client - User-Agent header being sent 2. Server - User-Agent header received in parseInput 3. Server - Platform detection in handlePublicMode 4. Server - applyUI platform type resolution 5. Client - Response _meta check for hasToolUI
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/sdk/src/auth/authorization/authorization.class.ts(2 hunks)libs/sdk/src/auth/machine-id.ts(1 hunks)libs/sdk/src/auth/session/utils/session-id.utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/auth/machine-id.tslibs/sdk/src/auth/authorization/authorization.class.tslibs/sdk/src/auth/session/utils/session-id.utils.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/auth/machine-id.tslibs/sdk/src/auth/authorization/authorization.class.tslibs/sdk/src/auth/session/utils/session-id.utils.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/auth/machine-id.tslibs/sdk/src/auth/authorization/authorization.class.tslibs/sdk/src/auth/session/utils/session-id.utils.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
🧬 Code graph analysis (3)
libs/sdk/src/auth/machine-id.ts (1)
libs/sdk/src/auth/authorization/authorization.class.ts (1)
getMachineId(19-19)
libs/sdk/src/auth/authorization/authorization.class.ts (1)
libs/sdk/src/auth/machine-id.ts (1)
getMachineId(23-25)
libs/sdk/src/auth/session/utils/session-id.utils.ts (3)
libs/sdk/src/common/types/auth/session.types.ts (1)
SessionIdPayload(71-86)libs/sdk/src/auth/authorization/authorization.class.ts (1)
getMachineId(19-19)libs/sdk/src/auth/machine-id.ts (1)
getMachineId(23-25)
🔇 Additional comments (6)
libs/sdk/src/auth/machine-id.ts (1)
19-25: LGTM!The getter implementation is clean and provides a stable machine ID for the process lifetime.
libs/sdk/src/auth/session/utils/session-id.utils.ts (3)
8-8: LGTM!The import of
getMachineIdfrom the centralized module and the updated comment correctly reflect the new single source of truth pattern.Also applies to: 14-14
15-18: LGTM!The
getKey()function correctly usesgetMachineId()for key derivation, maintaining consistency with the centralized machine ID module.
191-198: LGTM!The session payload correctly uses
getMachineId()for thenodeIdfield, ensuring consistency with session validation logic.libs/sdk/src/auth/authorization/authorization.class.ts (2)
16-16: LGTM!The import and re-export pattern correctly maintains backwards compatibility for external consumers who may be importing
getMachineIdfrom this module.Also applies to: 18-19
79-94: LGTM!The
createTransportSessionmethod correctly usesgetMachineId()for thenodeIdfield, ensuring consistency across all session-related operations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
libs/testing/src/fixtures/test-fixture.ts (2)
132-134: Clarify the "all logs" comment.The comment states "show all logs", but the code actually shows only the last 50 logs via
logs.slice(-50). This is a reasonable safeguard, but the comment should reflect this limitation.Apply this diff to clarify the comment:
- // On failure, show all logs + // On failure, show last 50 logs console.log('[DIAG:server-logs] No DIAG logs found, showing all logs:');Or update the console message:
// On failure, show all logs - console.log('[DIAG:server-logs] No DIAG logs found, showing all logs:'); + console.log('[DIAG:server-logs] No DIAG logs found, showing last 50 logs:');
264-277: Consider extracting the testFailed tracking pattern.The pattern of declaring
testFailed, setting it in the catch block, and passing it tocleanupTestFixturesis duplicated acrosstestWithFixtures,skip, andonly. While the duplication is minimal, extracting this pattern could improve maintainability.Consider creating a helper function:
async function runTestWithCleanup( name: string, fn: TestFn, jestFn: typeof it | typeof it.skip | typeof it.only ): void { jestFn(name, async () => { const fixtures = await createTestFixtures(); let testFailed = false; try { await fn(fixtures); } catch (error) { testFailed = true; throw error; } finally { await cleanupTestFixtures(fixtures, testFailed); } }); }Then simplify the three functions:
function testWithFixtures(name: string, fn: TestFn): void { runTestWithCleanup(name, fn, it); } function skip(name: string, fn: TestFn): void { runTestWithCleanup(name, fn, it.skip); } function only(name: string, fn: TestFn): void { runTestWithCleanup(name, fn, it.only); }Also applies to: 296-309, 314-327
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
386-398: Consider guarding diagnostic logs or using the existing logger.The new
console.log('[DIAG:...]')statements are not guarded by environment checks (unlike the existing debug logging at lines 572-578 which checksDEBUGorNODE_ENV). These will output in production and may create log noise.Options:
- Guard with environment check similar to the existing pattern
- Use the existing
this.logger.debug()orthis.logger.verbose()for consistent log levels- Remove before merging if these are temporary CI debugging aids
If these are meant to be permanent, consider using the existing logger:
- console.log('[DIAG:call-tool.applyUI] platform detection context', { + this.logger.debug('applyUI: platform detection context', { toolName: tool.metadata.name, sessionId: sessionId?.slice(0, 20), // ... rest of properties });Or guard them like the existing debug pattern:
+ if (process.env['DEBUG'] || process.env['NODE_ENV'] === 'development') { console.log('[DIAG:call-tool.applyUI] platform detection context', { // ... }); + }Also applies to: 408-418, 428-437, 448-456, 528-540, 624-633
libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts (1)
38-46: Same diagnostic logging pattern - consider guarding or using structured logger.Consistent with
call-tool.flow.ts, theseconsole.log('[DIAG:...]')statements will run in production. The file already has a structured logger (logger) created at line 25 - consider using it for consistency.- console.log('[DIAG:initialize] request received', { + logger.debug('initialize: request context', { clientInfo: request.params.clientInfo, hasCapabilities: !!request.params.capabilities, sessionId: ctx.authInfo?.sessionId?.slice(0, 20), // ... });Also applies to: 69-75, 95-104, 114-120, 122-128, 136-141, 143-148
libs/sdk/src/auth/flows/session.verify.flow.ts (1)
154-162: Consider guarding diagnostic logs in SDK library code.Same pattern as other files - unguarded
console.log('[DIAG:...]')statements. In SDK library code that may be used by many consumers, production logging noise is a stronger concern.Consider using the scope logger if available, or guard with environment check:
+ if (process.env['DEBUG'] || process.env['FRONTMCP_DEBUG']) { console.log('[DIAG:session.verify] handlePublicMode called', { // ... }); + }Also applies to: 171-180, 192-197, 223-229, 244-250
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts(2 hunks)libs/sdk/src/auth/flows/session.verify.flow.ts(7 hunks)libs/sdk/src/auth/machine-id.ts(1 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(6 hunks)libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts(4 hunks)libs/testing/src/client/mcp-test-client.ts(2 hunks)libs/testing/src/fixtures/test-fixture.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/testing/src/client/mcp-test-client.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/auth/machine-id.tslibs/sdk/src/auth/flows/session.verify.flow.tsapps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/testing/src/fixtures/test-fixture.tslibs/sdk/src/tool/flows/call-tool.flow.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/auth/machine-id.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/tool/flows/call-tool.flow.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/auth/machine-id.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/testing/src/fixtures/test-fixture.tslibs/sdk/src/tool/flows/call-tool.flow.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/testing/src/fixtures/test-fixture.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/testing/src/fixtures/test-fixture.ts
🧬 Code graph analysis (3)
libs/sdk/src/auth/machine-id.ts (1)
libs/sdk/src/auth/authorization/authorization.class.ts (1)
getMachineId(19-19)
libs/sdk/src/auth/flows/session.verify.flow.ts (2)
libs/sdk/src/auth/session/utils/session-id.utils.ts (2)
decryptPublicSession(97-112)encryptJson(30-39)libs/sdk/src/notification/notification.service.ts (1)
detectPlatformFromUserAgent(181-199)
libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts (1)
libs/sdk/src/auth/session/utils/session-id.utils.ts (1)
updateSessionPayload(224-247)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (6)
libs/testing/src/fixtures/test-fixture.ts (1)
121-137: LGTM! Diagnostic logging implementation is sound.The implementation correctly:
- Detects CI environment using the standard
CIenvironment variable- Guards against null
serverInstancebefore accessing logs- Prioritizes DIAG-prefixed logs for clarity
- Falls back to the last 50 logs when no DIAG logs are available
The logging enhancement will significantly improve debugging for test failures in CI environments.
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts (2)
14-22: LGTM - Diagnostic logging is appropriate for E2E tests.Environment and context logging at module load and in
beforeAllis valuable for debugging CI failures. The captured information (platform, node version, CI flag, timestamps) will help trace test failures.Also applies to: 30-41
587-710: Good diagnostic test coverage for platform detection chain.The new diagnostic tests provide valuable tracing for CI debugging:
- Full flow tracing for OpenAI with detailed state logging
- Platform persistence across multiple requests validates session caching
- Concurrent tool calls validates thread safety of platform detection
The conditional error logging (lines 616-621, 700-704) before assertions is a good pattern for debugging test failures.
libs/sdk/src/auth/machine-id.ts (1)
1-30: LGTM - Documentation addresses production concerns.The module now includes clear documentation (lines 14-17) about:
- Session invalidation on restart when
MACHINE_IDis not set- Production guidance for multi-instance deployments
- Session affinity considerations
This addresses the concerns raised in the previous review. The IIFE pattern ensures the ID is computed once and remains stable for the process lifetime.
libs/sdk/src/auth/flows/session.verify.flow.ts (2)
219-240: LGTM - Platform detection integration is well-implemented.The platform detection from User-Agent:
- Uses the configurable
platformDetectionConfigfrom scope metadata- Only includes
platformTypein session payload when it's not'unknown'- Propagates to subsequent requests via the encrypted session ID
The conditional spread at line 239 is clean and prevents payload bloat for unknown platforms.
183-210: LGTM - Secure session payload handling.The logic correctly:
- Always uses the client's session ID for transport registry lookup (line 205)
- Only trusts the decrypted payload when
nodeIdmatches current machine (line 190)- Responds with
finalPayload: undefinedwhen validation fails, letting the transport layer handle errors appropriatelyThis prevents session payload spoofing across different server instances.
The CI logs revealed that React component rendering is failing silently.s Added console.log in the catch block of applyUI to capture the actual error message when UI rendering fails on Linux.
Adds a manually-triggerable workflow to debug platform detection test failures. Allows running specific test patterns without full CI suite. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/debug-ui-tests.yml (2)
3-10: Document the lifecycle of this branch-specific workflow.This workflow is tied exclusively to the
fix-frontmcp-uibranch and will become inactive once the branch is merged or deleted. If this is a temporary debug workflow, consider adding a comment in the workflow file explaining its purpose and expected lifecycle. If this workflow should persist, update the trigger to includemainor other relevant branches.
38-45: Consider parallelizing builds for faster execution.The three library builds are currently sequential. For faster execution, you could use:
- name: Build dependencies run: | echo "Building libraries in parallel..." npx nx run-many --target=build --projects=ui,sdk,testing --parallel=3However, sequential execution with explicit echo statements may be preferable for a debug workflow where clarity is prioritized over speed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/debug-ui-tests.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
.github/workflows/debug-ui-tests.yml
⏰ 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). (3)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
- GitHub Check: Debug UI E2E Tests
🔇 Additional comments (4)
.github/workflows/debug-ui-tests.yml (4)
12-21: LGTM!The environment configuration is appropriate for CI. Disabling the NX daemon and setting a fixed
MACHINE_IDensures deterministic behavior for debugging session-related issues.
22-36: LGTM!The checkout and Node.js setup follow CI best practices. Using
.nvmrcfor version management, enabling yarn cache, and installing with--frozen-lockfileensures reproducible builds.
47-57: LGTM!Printing environment details is valuable for debugging and helps troubleshoot platform-specific issues. The output includes all relevant version and configuration information.
66-74: LGTM!The cleanup step follows CI best practices by running always and using
|| trueto prevent failures when no processes are found. The specific pattern matching (demo-e2e-ui) ensures only relevant processes are targeted, and showing remaining processes aids debugging.
…session verification
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/testing/src/fixtures/test-fixture.ts (1)
117-125: Implement the documented server log output or update the JSDoc.The JSDoc states that the
_testFailedparameter is used "to output server logs," but the parameter is not used in the function body. The underscore prefix conventionally indicates an intentionally unused parameter, creating an inconsistency with the documentation.Since this is a publishable SDK library, either implement the intended behavior or update the JSDoc to reflect that this is a placeholder for future enhancement.
If the intent is to output server logs on test failure, consider implementing:
-async function cleanupTestFixtures(fixtures: TestFixtures, _testFailed = false): Promise<void> { +async function cleanupTestFixtures(fixtures: TestFixtures, testFailed = false): Promise<void> { // Disconnect client if (fixtures.mcp.isConnected()) { await fixtures.mcp.disconnect(); } + // Output server logs on failure for debugging + if (testFailed && serverInstance) { + const logs = serverInstance.getLogs(); + if (logs.length > 0) { + console.error('Server logs from failed test:'); + console.error(logs); + } + } }
♻️ Duplicate comments (1)
libs/testing/src/fixtures/test-fixture.ts (1)
296-309: Same duplication pattern asskipfunction.The error handling is correct but duplicates the same pattern. See the previous comment on lines 278-291 for the optional refactoring suggestion.
🧹 Nitpick comments (2)
libs/testing/src/fixtures/test-fixture.ts (1)
278-291: Consider extracting the common error handling pattern.The error handling logic is correct and identical to
testWithFixtures. While functional, the duplication across three functions (testWithFixtures,skip,only) could be reduced by extracting a shared wrapper.Optional refactor to reduce duplication:
async function runTestWithFixtures(fn: TestFn): Promise<void> { const fixtures = await createTestFixtures(); let testFailed = false; try { await fn(fixtures); } catch (error) { testFailed = true; throw error; } finally { await cleanupTestFixtures(fixtures, testFailed); } } function testWithFixtures(name: string, fn: TestFn): void { it(name, () => runTestWithFixtures(fn)); } function skip(name: string, fn: TestFn): void { it.skip(name, () => runTestWithFixtures(fn)); } function only(name: string, fn: TestFn): void { it.only(name, () => runTestWithFixtures(fn)); }libs/sdk/src/tool/flows/call-tool.flow.ts (1)
495-510: Extract duplicated template type inference into a helper method.The template type detection logic (lines 500–508 in the console.log) is duplicated verbatim in the logger.error call below. Extract this into a private method to avoid maintenance issues:
private inferTemplateType(template: unknown): string { if (!template) return 'none'; if (typeof template === 'function') return 'react-component'; if (typeof template === 'string') { return template.endsWith('.tsx') || template.endsWith('.jsx') ? 'react-file' : 'html-file'; } return 'unknown'; }Then call it from both logging statements. This keeps the diagnostic logging intact while reducing duplication and improving maintainability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CLAUDE.md(2 hunks)libs/sdk/src/auth/flows/session.verify.flow.ts(6 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(1 hunks)libs/testing/src/client/mcp-test-client.ts(1 hunks)libs/testing/src/fixtures/test-fixture.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/auth/flows/session.verify.flow.tslibs/testing/src/fixtures/test-fixture.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/client/mcp-test-client.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/flows/call-tool.flow.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/auth/flows/session.verify.flow.tslibs/testing/src/fixtures/test-fixture.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/client/mcp-test-client.ts
🧠 Learnings (4)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/testing/src/fixtures/test-fixture.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
libs/testing/src/fixtures/test-fixture.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
🧬 Code graph analysis (1)
libs/sdk/src/auth/flows/session.verify.flow.ts (1)
libs/sdk/src/notification/notification.service.ts (1)
detectPlatformFromUserAgent(181-199)
⏰ 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). (3)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
- GitHub Check: Debug UI E2E Tests
🔇 Additional comments (7)
libs/testing/src/client/mcp-test-client.ts (1)
805-805: LGTM! clientInfo correctly propagated to transport.The addition of
clientInfoto the transport configuration properly enables the transport layer to access client metadata for User-Agent header construction, aligning with the PR objectives.libs/sdk/src/auth/flows/session.verify.flow.ts (5)
26-26: LGTM!Import of
detectPlatformFromUserAgentaligns with the platform detection usage below.
36-36: LGTM!Adding
userAgentto state schema follows the established pattern and correctly usesstate.set()for flow state management per coding guidelines.
112-123: LGTM!User-Agent extraction follows the established pattern and correctly propagates through
state.set(). Based on learnings, this properly avoids mutatingrawInput.
169-178: Verify behavior in multi-node deployments.The
finalPayloadlogic discards the existing session payload whennodeIddoesn't matchmachineId. In distributed environments where different nodes may handle the same session, this could cause payload data (protocol info, etc.) to be lost when requests are routed to a different node than the one that created the session.If this is intentional (e.g., for security or consistency), consider adding a brief comment explaining why payload from other nodes should not be trusted.
191-204: LGTM!Platform detection implementation is clean:
- Safely accesses optional config with
?.detectPlatformFromUserAgentgracefully handles undefined inputs- Conditional spread avoids adding 'unknown' to payload
Good use of existing state (
this.state.userAgent) rather than re-accessing the request.libs/testing/src/fixtures/test-fixture.ts (1)
246-259: LGTM!The error handling correctly tracks test failure status and propagates it to cleanup while maintaining proper error re-throwing for test failure reporting.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/ui/src/registry/tool-ui.registry.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/registry/tool-ui.registry.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/ui/src/registry/tool-ui.registry.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/ui/src/registry/tool-ui.registry.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/registry/tool-ui.registry.ts
⏰ 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: Build Libraries
- GitHub Check: Debug UI E2E Tests
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/debug-ui-tests.yml (1)
59-62: Add artifact collection for test failures.As noted in a previous review, when tests fail there are no artifacts captured (test logs, screenshots, or diagnostic output), which limits the debugging value of this workflow.
Consider adding a step after test execution to capture artifacts:
- name: Upload test artifacts on failure if: failure() uses: actions/upload-artifact@v4 with: name: test-artifacts path: | apps/e2e/demo-e2e-ui/test-results/ apps/e2e/demo-e2e-ui/screenshots/ apps/e2e/demo-e2e-ui/**/*.log retention-days: 7
🧹 Nitpick comments (1)
.github/workflows/debug-ui-tests.yml (1)
3-6: Temporary debug workflow—remove or generalize before merge.This workflow triggers only on the
fix-frontmcp-uibranch (the source branch of this PR), indicating it's intended for debugging during development. Ensure this workflow is either removed or generalized to apply to other relevant branches before merging to main.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/debug-ui-tests.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
.github/workflows/debug-ui-tests.yml
⏰ 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). (3)
- GitHub Check: Build Libraries
- GitHub Check: Lint & Format Checks
- GitHub Check: Debug UI E2E Tests
🔇 Additional comments (1)
.github/workflows/debug-ui-tests.yml (1)
64-72: LGTM! Good cleanup pattern with diagnostic output.The cleanup step appropriately uses
if: always()to ensure orphan processes are terminated regardless of test outcome. The pattern matching (tsx.*demo-e2e-uiandnode.*demo-e2e-ui) is specific enough to target only relevant processes, and the|| trueguards prevent workflow failure. The final diagnostic output is helpful for verifying cleanup success.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts (1)
50-68: Consider enhancing diagnostic robustness and consistency.The diagnostic logging is helpful for CI debugging. A few optional improvements:
Case-insensitive log filtering: Line 61 uses case-sensitive matching for 'DIAG' and 'react'. Consider using
.toLowerCase()to catch variations like 'React' or 'DIAG'.Safe JSON serialization: Line 53's
JSON.stringify(result.raw._meta)could throw on circular references. Wrap in a try-catch or use a safe stringify helper.Selective placement: Diagnostics appear only in this test (OpenAI variant with react-chart). If this is intentional for debugging a specific issue, consider adding a comment explaining why. Otherwise, consider extracting to a reusable helper if other tests would benefit.
Example for case-insensitive filtering:
- const reactLogs = logs.filter((l) => l.includes('DIAG') || l.includes('react')); + const reactLogs = logs.filter((l) => { + const lower = l.toLowerCase(); + return lower.includes('diag') || lower.includes('react'); + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts
⏰ 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). (3)
- GitHub Check: Build Libraries
- GitHub Check: Lint & Format Checks
- GitHub Check: Debug UI E2E Tests
There was a problem hiding this comment.
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/ui/src/adapters/index.ts (1)
24-34: Breaking API change in public adapter exports:platformUsesDualPayloadrenamed toplatformUsesStructuredContent.Consumers of
@frontmcp/uiimportingplatformUsesDualPayloadwill encounter a breaking change. While the documentation discusses the dual-payload/structured-content concept across multiple guides, there is no API reference documenting the adapter module exports. Per coding guidelines, public API changes inlibs/**require matchingdocs/draft/docs/**updates. An API reference section for@frontmcp/ui/adaptersshould be added or updated to document the renamed function and its purpose.
🧹 Nitpick comments (1)
libs/ui/src/adapters/response-builder.ts (1)
179-187: Widget fallback path may be unreachable with current configuration.Currently, all platforms with
supportsWidgets: truealso haveuseStructuredContent: trueinPLATFORM_CAPABILITIES. This means the widget fallback path (lines 181-186) won't execute under the current configuration.Consider whether this code path should be kept for future flexibility or documented as intentionally unreachable with current platform mappings. If it's intentional to preserve for future platforms that support widgets but not structuredContent, adding a comment would clarify this.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
libs/sdk/src/tool/flows/call-tool.flow.ts(5 hunks)libs/ui/src/adapters/__tests__/dual-payload.test.ts(0 hunks)libs/ui/src/adapters/__tests__/response-builder.test.ts(8 hunks)libs/ui/src/adapters/__tests__/serving-mode.test.ts(5 hunks)libs/ui/src/adapters/dual-payload.ts(0 hunks)libs/ui/src/adapters/index.ts(1 hunks)libs/ui/src/adapters/response-builder.ts(6 hunks)libs/ui/src/adapters/serving-mode.ts(10 hunks)libs/ui/src/index.ts(2 hunks)
💤 Files with no reviewable changes (2)
- libs/ui/src/adapters/tests/dual-payload.test.ts
- libs/ui/src/adapters/dual-payload.ts
🧰 Additional context used
📓 Path-based instructions (7)
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/response-builder.tslibs/ui/src/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/response-builder.tslibs/ui/src/index.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/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/response-builder.tslibs/ui/src/index.ts
libs/ui/src/**/*.test.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.test.ts: Maintain 95%+ test coverage across statements, branches, functions, and lines
Test HTML escaping for user-provided content to prevent XSS attacks
Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Files:
libs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
libs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/tool/flows/call-tool.flow.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Files:
libs/ui/src/index.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `getCapabilities()` for dynamic capability exposure instead of hardcoding capabilities in adapters
Applied to files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/index.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
Applied to files:
libs/ui/src/adapters/index.tslibs/ui/src/adapters/serving-mode.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
libs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
libs/ui/src/adapters/__tests__/serving-mode.test.tslibs/ui/src/adapters/__tests__/response-builder.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
libs/ui/src/adapters/__tests__/serving-mode.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
libs/ui/src/adapters/__tests__/response-builder.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
libs/ui/src/index.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/common/records/**/*.ts : Centralize record types in common/records and import from there instead of from module-specific files
Applied to files:
libs/ui/src/index.ts
🧬 Code graph analysis (3)
libs/ui/src/adapters/__tests__/response-builder.test.ts (1)
libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)
libs/sdk/src/tool/flows/call-tool.flow.ts (2)
libs/ui/src/registry/tool-ui.registry.ts (1)
isUIRenderFailure(91-93)libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)
libs/ui/src/adapters/serving-mode.ts (2)
libs/ui/src/adapters/index.ts (2)
AIPlatformType(13-13)platformUsesStructuredContent(32-32)libs/ui/src/index.ts (2)
AIPlatformType(51-51)platformUsesStructuredContent(66-66)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (22)
libs/ui/src/adapters/serving-mode.ts (4)
26-49: Clean interface design for the structuredContent transition.The
ResolvedServingModeinterface is well-structured with clear documentation. The addition ofsupportsUIalongsideuseStructuredContentprovides good separation of concerns - widget support vs. content format.
89-150: Platform capabilities are consistently mapped.The
PLATFORM_CAPABILITIESmap correctly alignsuseStructuredContentwith widget support across all platforms. The pattern ofsupportsWidgets: true⟺useStructuredContent: trueis consistent, which makes the behavior predictable.
185-228: Resolver logic correctly propagates structuredContent flag.All four code paths in
resolveServingModecorrectly returnuseStructuredContentfrom the platform capabilities. The fallback toPLATFORM_CAPABILITIES.unknownon line 187 ensures graceful handling of unexpected platform types.
251-259: Helper function correctly renamed and documented.The
platformUsesStructuredContentfunction follows the established pattern and provides clear documentation about the structuredContent format behavior.libs/ui/src/adapters/__tests__/serving-mode.test.ts (4)
6-11: Import updated correctly for renamed function.The import correctly reflects the API change from
platformUsesDualPayloadtoplatformUsesStructuredContent.
15-115: Comprehensive auto mode test coverage across all platforms.Tests cover all defined platforms (openai, ext-apps, claude, cursor, continue, cody, generic-mcp, gemini, unknown) with correct assertions for
useStructuredContentbehavior. This aligns with the coding guidelines to test behavior across platform configurations. Based on learnings.
117-191: Forced mode tests correctly validate structuredContent behavior.The tests properly verify that
useStructuredContentis returned when a supported mode is explicitly configured (line 167), and that UI is skipped when unsupported modes are forced on platforms like Claude and Continue.
243-258: Test suite forplatformUsesStructuredContentcovers all platforms.The tests comprehensively verify the helper function returns
truefor all widget-supporting platforms andfalsefor non-widget platforms (gemini, unknown).libs/sdk/src/tool/flows/call-tool.flow.ts (3)
27-28: New import for UI render failure handling.The
isUIRenderFailureimport enables proper discrimination of render failures for graceful degradation.
464-477: Graceful degradation for UI render failures is well-implemented.The handling correctly logs a warning with useful context (tool, reason, platform) and proceeds without UI content. This ensures tool functionality isn't blocked by UI rendering issues, which aligns with the existing error handling approach in the outer try-catch (lines 501-530).
415-500: Correct propagation ofuseStructuredContentthrough applyUI.The
useStructuredContentflag is properly extracted from the resolved mode (line 416) and passed tobuildToolResponseContent(line 485). The logging at line 497 aids debugging.libs/ui/src/adapters/response-builder.ts (3)
33-59: Clean interface update with proper typing.
BuildToolResponseOptionscorrectly usesuseStructuredContent: booleaninstead of the olduseDualPayload. The documentation clearly explains the behavior when true.
64-92:ToolResponseContentinterface properly typed.The
structuredContent?: unknowntype follows coding guidelines to useunknowninstead ofany. The updated format union removesdual-payloadand addsstructured-contentto reflect the new semantics.
130-177: Inline mode structuredContent logic is correct.The function properly handles:
useStructuredContent: true+ HTML →structured-contentformat with raw HTML in content and rawOutput in structuredContentuseStructuredContent: true+ no HTML →json-onlyformat with structuredContent populated- Empty string HTML is correctly treated as falsy (line 159)
libs/ui/src/adapters/__tests__/response-builder.test.ts (6)
10-16: Test helper correctly defaults to new API.The
createOptionshelper setsuseStructuredContent: trueas the default, aligning with the typical expected usage for widget-supporting platforms.
34-58: Static mode structuredContent tests are well-structured.Tests properly verify that:
structuredContentis included whenuseStructuredContent: true(lines 34-45)structuredContentis undefined whenuseStructuredContent: false(lines 47-58)
119-183: Comprehensive inline mode tests across all widget-supporting platforms.The parameterized test (lines 130-152) efficiently covers all 7 widget-supporting platforms. Fallback behavior tests (lines 154-182) verify correct handling when HTML content is missing or empty. Based on learnings.
185-200: Widget fallback test covers legacy behavior.The test verifies that when
useStructuredContent: falseon a widget-supporting platform, the legacy widget format is used with cleared content.
406-445: Dedicated structuredContent property tests cover all combinations.The test suite provides focused coverage for the
structuredContentproperty:
- With HTML (line 407-418)
- Without HTML (line 420-431)
- When disabled (line 433-444)
246-260: Test correctly verifies HTML passthrough for client-side sandbox rendering.The test accurately documents the intended security model: HTML is passed as-is because the structured-content format is designed for client-side sandbox rendering on the platform (OpenAI, Claude, etc.). This is a layered security approach where content protection relies on iframe sandboxing, not server-side HTML escaping. The
htmlContentrepresents tool output (not user-provided input), which is appropriate for structured-content rendering. The coding guideline to useescapeHtml()applies to user-provided content inlibs/ui/src/**/*.ts(form inputs, user text, labels), but htmlContent follows a different security pattern—trusted tool output rendered in an isolated sandbox context.libs/ui/src/index.ts (2)
342-463: Well-organized expansion of the public API surface.The new Universal Renderer Module and TypeScript Type Fetching Engine exports are well-documented and logically organized. The structure follows existing patterns in the file and maintains good separation of concerns.
66-66: No action required. Migration fromplatformUsesDualPayloadtoplatformUsesStructuredContentis complete—no remaining references to the old function name exist in the codebase, and the new function is properly exported, tested, and implemented.
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 (9)
libs/ui/src/components/badge.ts (2)
132-132: XSS vulnerability: icon parameter is not escaped.The
iconparameter is directly injected into the HTML without escaping at Line 132. If user-provided content is passed as the icon, this creates an XSS vulnerability. All user-provided content must be escaped.Based on coding guidelines: "Always use
escapeHtml()utility for all user-provided content to prevent XSS vulnerabilities".🔎 Apply this diff to escape the icon content:
- const iconHtml = icon ? `<span class="mr-1">${icon}</span>` : ''; + const iconHtml = icon ? `<span class="mr-1">${escapeHtml(icon)}</span>` : '';Note: If the icon is intentionally expected to contain trusted HTML (e.g., SVG markup), document this assumption clearly and consider adding a separate parameter like
iconHtmlfor pre-sanitized HTML while keepingiconfor plain text.
88-97: Missing schema validation per coding guidelines.The badge function does not validate the
optionsparameter using theBadgeOptionsSchemaandvalidateOptions()helper. According to the coding guidelines, components should validate inputs and return an error box on validation failure.Based on coding guidelines: "Validate component options using
validateOptions()helper and return error box on validation failure".Consider adding validation at the beginning of the function:
export function badge(text: string, options: BadgeOptions = {}): string { const validationResult = validateOptions(BadgeOptionsSchema, options); if (!validationResult.success) { return errorBox('Invalid badge options', validationResult.error); } const { variant = 'default', // ... rest of destructuring } = options; // ... rest of implementation }libs/ui/src/components/table.ts (1)
383-392: Potential XSS vulnerability in page size selector.The
baseUrlis interpolated directly into theonchangeJavaScript handler without escaping. IfbaseUrlcontains quotes or malicious content, it could break the handler or enable script injection.🔎 Apply this diff to escape the URL:
const pageSizeHtml = showPageSize ? `<select class="ml-4 px-2 py-1 text-sm border border-border rounded-lg bg-white" - onchange="window.location.href = '${baseUrl}${baseUrl.includes('?') ? '&' : '?'}page=1&pageSize=' + this.value" + onchange="window.location.href = '${escapeHtml(baseUrl)}${baseUrl.includes('?') ? '&' : '?'}page=1&pageSize=' + this.value" >libs/ui/src/components/table.schema.ts (1)
122-147: Schema missingbaseUrlfield - inconsistent with implementation.The
PaginationOptionsinterface intable.ts(lines 293-294) includes abaseUrl?: stringproperty, butPaginationOptionsSchemadoesn't define it. With.strict()mode, passingbaseUrlwill cause validation to fail.🔎 Apply this diff to add the missing field:
export const PaginationOptionsSchema = z .object({ /** Current page (1-indexed) */ page: z.number().min(1), /** Total pages */ totalPages: z.number().min(0), /** Total items count */ totalItems: z.number().min(0).optional(), /** Items per page */ pageSize: z.number().min(1).optional(), /** Page size options */ pageSizeOptions: z.array(z.number()).optional(), /** Show first/last buttons */ showFirstLast: z.boolean().optional(), /** Show page count */ showPageCount: z.boolean().optional(), /** Show item count */ showItemCount: z.boolean().optional(), /** Show page size selector */ showPageSize: z.boolean().optional(), /** Max visible page buttons */ maxVisiblePages: z.number().min(1).optional(), /** Additional CSS classes */ className: z.string().optional(), + /** Base URL for page links */ + baseUrl: z.string().optional(), }) .strict();libs/ui/src/components/alert.ts (2)
98-141: Add option validation per coding guidelines.The coding guidelines require validating component options using the
validateOptions()helper and returning an error box on validation failure. This pattern should be implemented at the start of thealert()function.As per coding guidelines: "Validate component options using
validateOptions()helper and return error box on validation failure" applies tolibs/ui/src/components/**/*.ts.
21-38: Update alert documentation and add input validation.
Breaking API change not documented: The
onDismissproperty has been removed fromAlertOptions, butdocs/draft/docs/ui/components/alert.mdxstill references it (lines 100, 255) and includes example code using this removed option. Update the documentation to remove references toonDismissand document the migration path for existing code.Missing input validation: Although
alert.schema.tsexists withAlertOptionsSchema, thealert()andtoast()functions do not validate options usingvalidateOptions(). Add validation to match the pattern used in other components likebutton.ts.Missing JSDoc @example tags: Add
@exampletags to thealert()andtoast()function documentation showing basic usage patterns and common options, as required by the component guidelines.libs/ui/src/widgets/resource.ts (2)
63-71: Remove unused HTMX properties from the interface.The
htmxproperties inResourceActionare no longer used anywhere in the codebase. Lines 252-260 compute HTMX attributes but never pass them to the button component, making these interface properties dead code.🔎 Apply this diff to remove the unused properties:
export interface ResourceAction { /** Action label */ label: string; /** Action icon */ icon?: string; /** Action URL */ href?: string; - /** HTMX attributes */ - htmx?: { - get?: string; - post?: string; - delete?: string; - target?: string; - swap?: string; - confirm?: string; - }; /** Variant */ variant?: 'primary' | 'secondary' | 'danger' | 'ghost'; /** Disabled */ disabled?: boolean; }
252-260: Remove dead code that computes unused HTMX attributes.The
htmxAttrsarray is built but never used. It's not passed to thebutton()function below (lines 262-268), making this entire block dead code.🔎 Apply this diff to remove the dead code:
const variant = action.variant ? variantMap[action.variant] : 'ghost'; - const htmxAttrs: string[] = []; - if (action.htmx) { - if (action.htmx.get) htmxAttrs.push(`hx-get="${escapeHtml(action.htmx.get)}"`); - if (action.htmx.post) htmxAttrs.push(`hx-post="${escapeHtml(action.htmx.post)}"`); - if (action.htmx.delete) htmxAttrs.push(`hx-delete="${escapeHtml(action.htmx.delete)}"`); - if (action.htmx.target) htmxAttrs.push(`hx-target="${escapeHtml(action.htmx.target)}"`); - if (action.htmx.swap) htmxAttrs.push(`hx-swap="${escapeHtml(action.htmx.swap)}"`); - if (action.htmx.confirm) htmxAttrs.push(`hx-confirm="${escapeHtml(action.htmx.confirm)}"`); - } - return button(action.label, { variant: variant as 'primary' | 'secondary' | 'danger' | 'ghost',libs/sdk/src/auth/ui/templates.ts (1)
94-182: Form submission implementation is broken and will not work in production.The consent page form POSTs to
/oauth/consentendpoint which does not exist. Additionally, the actual callback handler at/oauth/callbackis configured for GET requests only and reads parameters fromrequest.query, not from the POST request body. The form data (consent selections) will be completely lost.Critical issues:
- Missing endpoint: Form POSTs to
/oauth/consentbut no such endpoint exists- POST body not parsed:
oauth.callback.flow.tsonly reads fromrequest.query(lines 103-126), ignoring the POST body entirely- Data loss: Consent checkbox selections sent in the POST body will never be processed
- No E2E test coverage: No tests found for form submission workflows
Required fixes:
- Implement
/oauth/consentPOST endpoint or update form to POST to/oauth/callbackwith proper HTTP method configuration- Add POST body parsing to extract form data (
pending_auth_id,toolscheckbox selections)- Add E2E tests covering the complete consent form submission flow
- Ensure form data validation and error handling
🧹 Nitpick comments (7)
libs/ui/src/components/badge.schema.ts (1)
27-36: Variant enums are consistent with codebase patterns.The variant enum includes most standard variants. Consider adding
'ghost'to align fully with the coding guideline's recommended variant set, though the current set is reasonable for badge use cases.Based on coding guidelines.
libs/ui/src/web-components/core/base-element.test.ts (1)
384-396: Consider verifying absence of HTMX attributes.The test verifies that expected attributes are present but doesn't explicitly check that
hx-get,hx-post, and other HTMX attributes are NOT included. Consider adding assertions to verify the complete removal of HTMX attribute support.🔎 Suggested test enhancement
it('should return observed attributes from schema', () => { const attrs = TestElement.observedAttributes; expect(attrs).toContain('variant'); expect(attrs).toContain('size'); expect(attrs).toContain('disabled'); expect(attrs).toContain('full-width'); // Common attrs expect(attrs).toContain('class'); expect(attrs).toContain('id'); + // Verify HTMX attributes are not included + expect(attrs).not.toContain('hx-get'); + expect(attrs).not.toContain('hx-post'); });libs/ui/src/web-components/core/attribute-parser.ts (1)
130-148: Refactor to follow Zod 4 best practices and narrow the type signature.The function currently accepts the generic
ZodSchema<T>but only reliably works with object schemas. While the defensive check prevents crashes, it masks the true constraint. For Zod 4, library authors should useschema._zod.def.shapefor schema introspection rather than direct property access.Since all current usages pass
z.object()schemas, improve the implementation by:
- Narrowing the type signature to explicitly accept only
ZodObjectschemas- Using Zod 4-compliant introspection (
._zod.def.shapewhen needed for Zod 4/Mini compatibility)- Making the schema type constraint explicit in the API
This prevents future misuse if non-object schemas are accidentally passed, and aligns with Zod 4's recommended library author patterns.
libs/ui/src/components/card.schema.ts (1)
27-42: Variant and size enums deviate from standard naming conventions.Per coding guidelines, variants should use
('primary', 'secondary', 'outline', 'ghost', 'danger', 'success')and sizes should include('xs', 'sm', 'md', 'lg', 'xl'). The card component uses semantic variants like'default','outlined','elevated','filled'which are more appropriate for card styling, and omits'xs'/'xl'sizes.If this is intentional for card-specific semantics, consider documenting the rationale. Otherwise, align with the standard enums for consistency.
libs/ui/src/components/card.ts (1)
92-107: Missing JSDoc@exampletags.Per coding guidelines, components should include JSDoc examples with
@exampletags showing basic usage and options patterns. Consider adding documentation for thecard()andcardGroup()functions.Example documentation:
+/** + * Build a card component + * + * @example + * ```typescript + * // Basic card + * card('<p>Content</p>', { title: 'My Card' }); + * + * // Elevated card with footer + * card('<p>Content</p>', { + * variant: 'elevated', + * title: 'Card Title', + * footer: '<button>Action</button>' + * }); + * ``` + */ export function card(content: string, options: CardOptions = {}): string {libs/ui/src/components/alert.ts (2)
66-97: Add JSDoc examples for component functions.The coding guidelines require adding
@exampletags showing basic usage and options patterns for all components. Consider adding examples to the JSDoc foralert(),toast(), andtoastContainer()functions.As per coding guidelines: "Add JSDoc examples with @example tags showing basic usage and options patterns for all components" applies to
libs/ui/src/components/**/*.ts.Example JSDoc pattern:
/** * Build an alert component + * + * @example + * ```ts + * alert('Operation successful', { variant: 'success', dismissible: true }) + * ``` + * + * @example + * ```ts + * alert('Warning message', { + * variant: 'warning', + * title: 'Important', + * showIcon: true + * }) + * ``` */ export function alert(message: string, options: AlertOptions = {}): string {Also applies to: 98-141, 186-243
37-37: Document escaping expectations for actions parameter.The
actionsparameter accepts HTML strings that are inserted directly without escaping (line 126). Consider documenting that this parameter should contain pre-escaped HTML or HTML generated by trusted component functions to prevent XSS vulnerabilities.Example documentation:
/** Alert ID */ id?: string; - /** Actions (buttons) */ + /** Actions (buttons). Should contain pre-escaped HTML or HTML from trusted component functions. */ actions?: string; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
libs/sdk/src/auth/ui/__tests__/base-layout.test.ts(0 hunks)libs/sdk/src/auth/ui/__tests__/htmx-templates.test.ts(0 hunks)libs/sdk/src/auth/ui/base-layout.ts(0 hunks)libs/sdk/src/auth/ui/index.ts(2 hunks)libs/sdk/src/auth/ui/templates.ts(4 hunks)libs/ui/src/components/alert.schema.ts(0 hunks)libs/ui/src/components/alert.test.ts(0 hunks)libs/ui/src/components/alert.ts(2 hunks)libs/ui/src/components/badge.schema.ts(1 hunks)libs/ui/src/components/badge.test.ts(0 hunks)libs/ui/src/components/badge.ts(1 hunks)libs/ui/src/components/button.schema.ts(1 hunks)libs/ui/src/components/button.test.ts(0 hunks)libs/ui/src/components/button.ts(1 hunks)libs/ui/src/components/card.schema.ts(1 hunks)libs/ui/src/components/card.test.ts(0 hunks)libs/ui/src/components/card.ts(1 hunks)libs/ui/src/components/form.schema.ts(0 hunks)libs/ui/src/components/form.test.ts(0 hunks)libs/ui/src/components/form.ts(1 hunks)libs/ui/src/components/index.ts(0 hunks)libs/ui/src/components/list.schema.ts(1 hunks)libs/ui/src/components/list.test.ts(0 hunks)libs/ui/src/components/list.ts(1 hunks)libs/ui/src/components/modal.schema.ts(1 hunks)libs/ui/src/components/modal.ts(4 hunks)libs/ui/src/components/table.schema.ts(1 hunks)libs/ui/src/components/table.ts(8 hunks)libs/ui/src/web-components/core/attribute-parser.test.ts(0 hunks)libs/ui/src/web-components/core/attribute-parser.ts(3 hunks)libs/ui/src/web-components/core/base-element.test.ts(1 hunks)libs/ui/src/web-components/core/base-element.ts(0 hunks)libs/ui/src/web-components/elements/fmcp-button.ts(0 hunks)libs/ui/src/web-components/elements/fmcp-input.ts(0 hunks)libs/ui/src/web-components/elements/fmcp-select.ts(0 hunks)libs/ui/src/widgets/resource.ts(1 hunks)
💤 Files with no reviewable changes (17)
- libs/ui/src/components/alert.test.ts
- libs/ui/src/components/button.test.ts
- libs/ui/src/web-components/core/base-element.ts
- libs/ui/src/components/alert.schema.ts
- libs/ui/src/components/list.test.ts
- libs/ui/src/components/card.test.ts
- libs/ui/src/web-components/core/attribute-parser.test.ts
- libs/ui/src/components/index.ts
- libs/ui/src/components/form.test.ts
- libs/ui/src/components/form.schema.ts
- libs/ui/src/components/badge.test.ts
- libs/ui/src/web-components/elements/fmcp-select.ts
- libs/sdk/src/auth/ui/tests/htmx-templates.test.ts
- libs/ui/src/web-components/elements/fmcp-input.ts
- libs/ui/src/web-components/elements/fmcp-button.ts
- libs/sdk/src/auth/ui/tests/base-layout.test.ts
- libs/sdk/src/auth/ui/base-layout.ts
🧰 Additional context used
📓 Path-based instructions (9)
libs/ui/src/components/**/*.schema.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/components/**/*.schema.ts: Every component must have a Zod schema with.strict()mode to reject unknown properties
Use consistent enum naming for variants ('primary', 'secondary', 'outline', 'ghost', 'danger', 'success') and sizes ('xs', 'sm', 'md', 'lg', 'xl')
Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.ts
libs/ui/src/components/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/components/**/*.ts: Validate component options usingvalidateOptions()helper and return error box on validation failure
Add JSDoc examples with @example tags showing basic usage and options patterns for all components
Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/card.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/table.tslibs/ui/src/components/modal.tslibs/ui/src/components/badge.tslibs/ui/src/components/form.tslibs/ui/src/components/button.ts
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/ui/src/components/badge.schema.tslibs/ui/src/web-components/core/attribute-parser.tslibs/ui/src/components/card.tslibs/ui/src/widgets/resource.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/table.tslibs/ui/src/components/modal.tslibs/ui/src/components/badge.tslibs/ui/src/components/form.tslibs/ui/src/components/button.tslibs/ui/src/web-components/core/base-element.test.ts
libs/ui/src/components/**
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Organize components into schema.ts (definitions) and implementation .ts files with matching names
Files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/card.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/table.tslibs/ui/src/components/modal.tslibs/ui/src/components/badge.tslibs/ui/src/components/form.tslibs/ui/src/components/button.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/ui/src/components/badge.schema.tslibs/sdk/src/auth/ui/templates.tslibs/ui/src/web-components/core/attribute-parser.tslibs/ui/src/components/card.tslibs/ui/src/widgets/resource.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/sdk/src/auth/ui/index.tslibs/ui/src/components/modal.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/table.tslibs/ui/src/components/modal.tslibs/ui/src/components/badge.tslibs/ui/src/components/form.tslibs/ui/src/components/button.tslibs/ui/src/web-components/core/base-element.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/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/ui/src/components/badge.schema.tslibs/sdk/src/auth/ui/templates.tslibs/ui/src/web-components/core/attribute-parser.tslibs/ui/src/components/card.tslibs/ui/src/widgets/resource.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/sdk/src/auth/ui/index.tslibs/ui/src/components/modal.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/table.tslibs/ui/src/components/modal.tslibs/ui/src/components/badge.tslibs/ui/src/components/form.tslibs/ui/src/components/button.tslibs/ui/src/web-components/core/base-element.test.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/auth/ui/templates.tslibs/sdk/src/auth/ui/index.ts
libs/ui/src/**/*.test.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.test.ts: Maintain 95%+ test coverage across statements, branches, functions, and lines
Test HTML escaping for user-provided content to prevent XSS attacks
Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Files:
libs/ui/src/web-components/core/base-element.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
libs/ui/src/web-components/core/base-element.test.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/ui/src/components/badge.schema.tslibs/sdk/src/auth/ui/templates.tslibs/ui/src/web-components/core/attribute-parser.tslibs/ui/src/components/card.tslibs/ui/src/widgets/resource.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/sdk/src/auth/ui/index.tslibs/ui/src/components/modal.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/table.tslibs/ui/src/components/modal.tslibs/ui/src/components/badge.tslibs/ui/src/components/form.tslibs/ui/src/components/button.tslibs/ui/src/web-components/core/base-element.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use consistent enum naming for variants ('primary', 'secondary', 'outline', 'ghost', 'danger', 'success') and sizes ('xs', 'sm', 'md', 'lg', 'xl')
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/modal.schema.tslibs/ui/src/web-components/core/base-element.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/list.tslibs/sdk/src/auth/ui/templates.tslibs/ui/src/components/card.tslibs/ui/src/components/form.tslibs/ui/src/components/button.tslibs/ui/src/web-components/core/base-element.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Use Zod schema validation for all component inputs as a core validation strategy
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/form.tslibs/ui/src/web-components/core/base-element.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.tslibs/ui/src/components/table.schema.tslibs/ui/src/components/button.schema.tslibs/ui/src/components/alert.tslibs/ui/src/components/form.tslibs/ui/src/web-components/core/base-element.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/table.schema.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
libs/ui/src/components/card.schema.tslibs/ui/src/components/badge.schema.tslibs/ui/src/components/list.schema.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/sdk/src/auth/ui/index.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Applied to files:
libs/sdk/src/auth/ui/index.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
libs/ui/src/web-components/core/base-element.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/ui/src/web-components/core/base-element.test.ts
🧬 Code graph analysis (2)
libs/ui/src/components/card.ts (2)
libs/sdk/src/auth/ui/templates.ts (1)
escapeHtml(87-87)libs/sdk/src/auth/ui/base-layout.ts (1)
escapeHtml(165-173)
libs/ui/src/components/modal.ts (2)
libs/sdk/src/auth/ui/templates.ts (1)
escapeHtml(87-87)libs/sdk/src/auth/ui/base-layout.ts (1)
escapeHtml(165-173)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (27)
libs/ui/src/components/form.ts (1)
585-601: LGTM! Clean removal of HTMX support from the form wrapper.The destructuring at line 586 correctly excludes HTMX-related options, and the attribute construction (lines 588-598) appropriately omits HTMX attributes. This is consistent with the broader PR objective of removing HTMX integrations across UI components.
libs/ui/src/components/badge.schema.ts (2)
46-46: Size enums are appropriate for badge component.The size enum (
'sm' | 'md' | 'lg') is reasonable for badges. While the coding guidelines suggest'xs' | 'sm' | 'md' | 'lg' | 'xl', the narrower range makes sense given badge dimensions.Based on coding guidelines.
60-77: HTMX removal aligns with PR objectives; schema properly configured.The removal of the
onRemovefield andBadgeOnRemoveSchemasuccessfully eliminates HTMX-specific configuration from the badge component, consistent with the broader shift described in the PR summary. The schema correctly applies.strict()mode to reject unknown properties.Based on coding guidelines and PR objectives.
libs/ui/src/components/badge.ts (1)
134-145: Inline onclick handler is a pragmatic replacement for HTMX.The shift from HTMX attributes to a simple inline
onclick="this.parentElement.remove()"handler successfully eliminates the HTMX dependency while maintaining removable badge functionality. The approach is straightforward and appropriate for this use case.libs/ui/src/web-components/core/base-element.test.ts (1)
171-171: LGTM: Consistent with HTMX removal.The simplified HTML rendering without HTMX attributes aligns with the broader refactoring to remove HTMX functionality across the codebase.
libs/ui/src/web-components/core/attribute-parser.ts (2)
6-6: LGTM: Documentation accurately reflects HTMX removal.The updated comments correctly remove references to HTMX functionality, focusing on the remaining attribute handling capabilities (boolean attributes, data attributes, kebab-to-camel conversion).
Also applies to: 153-153
166-174: LGTM: Simplified logic after HTMX removal.The condition now only checks for data attributes, which is correct after removing HTMX-specific handling. The logic properly nests data attributes under the
datakey while assigning other attributes directly.libs/ui/src/components/card.schema.ts (1)
51-76: LGTM! Schema correctly uses.strict()mode.The CardOptionsSchema properly enforces strict validation to reject unknown properties, aligning with coding guidelines. The HTMX field removal is clean.
libs/ui/src/components/card.ts (1)
134-145: LGTM! HTMX attributes cleanly removed.The anchor and div rendering paths now correctly exclude HTMX attributes. All user-provided content (
href,id,datavalues,title,subtitle) is properly escaped withescapeHtml().libs/ui/src/components/table.ts (3)
109-142: LGTM! Header rendering cleanly updated.HTMX attributes removed from sortable headers. The sort indicator rendering with
buildSortIndicator()provides a clean visual indication of sort state. Header text is properly escaped.
155-160: LGTM! Body rendering simplified.Destructuring cleanly removes HTMX-related fields. Row selection checkbox no longer emits HTMX attributes, aligning with the broader migration away from HTMX.
293-295: NewbaseUrlproperty added to PaginationOptions.This is a public API addition. Ensure documentation is updated to reflect this new option for URL-based pagination.
libs/ui/src/components/table.schema.ts (1)
42-63: Usingz.any()for render function is acceptable.The comment correctly documents that function validation isn't possible at runtime with Zod. Consider using
z.function()if Zod 4's function schema improvements apply, thoughz.any()is pragmatic here.libs/ui/src/components/list.schema.ts (2)
170-185: LGTM! ActionItemSchema cleanly updated.HTMX field removed while maintaining
.strict()mode. The schema continues to properly validate action item properties.
1-207: All schemas correctly use.strict()mode.All list-related schemas (
PermissionItemSchema,FeatureItemSchema,DescriptionItemSchema,ActionItemSchema, and their options schemas) properly enforce strict validation per coding guidelines.libs/ui/src/components/button.schema.ts (1)
1-123: LGTM! Clean HTMX removal with proper schema structure.The ButtonOptionsSchema correctly follows coding guidelines with
.strict()mode and consistent enum naming conventions. The removal of HTMX-related fields is complete and well-documented.Based on coding guidelines: "Every component must have a Zod schema with
.strict()mode to reject unknown properties" and "Use consistent enum naming for variants ('primary', 'secondary', 'outline', 'ghost', 'danger', 'success') and sizes ('xs', 'sm', 'md', 'lg', 'xl')."libs/ui/src/components/list.ts (2)
368-381: LGTM! ActionItem interface correctly simplified.The removal of the
htmxproperty fromActionItemis clean and aligns with the broader HTMX removal effort across the codebase.
386-428: LGTM! Clean HTML rendering without HTMX.The
actionListfunction now renders pure HTML without HTMX attributes, correctly usingescapeHtmlfor user-provided content to prevent XSS vulnerabilities.Based on learnings: "Use pure HTML string generation without React/Vue/JSX - components return HTML strings only."
libs/ui/src/components/button.ts (1)
179-293: LGTM! Excellent security implementation.The button function correctly:
- Validates options using Zod schema
- Validates href protocols with allowlist approach (lines 138-149, 224-227)
- Escapes all user-provided content
- Sanitizes data attribute keys
- Removes all HTMX integration cleanly
The security considerations in the JSDoc are thorough and accurate.
libs/ui/src/components/modal.schema.ts (3)
44-65: Breaking change:onClosefield removal is complete.The removal of the
onClosefield fromModalOptionsSchemahas been fully implemented across the codebase with no remaining references toonCloseor theModalOnClosetype. TheModalOptionsinterface matches the schema with.strict()mode enabled, confirming all consumer code has been updated.
89-113: No migration needed:onConfirmfield does not exist in the codebase.Search results show zero references to
onConfirmorConfirmModalOnConfirmtype across the entire repository. The schema currently usesconfirmHreffor URL-based confirm actions, which is correctly implemented in the modal component. However, theConfirmModalOptionsSchemalacks HTMX properties specified in coding guidelines—addtarget,swap, andtriggerproperties to align with the HTMX schema pattern (get, post, put, delete, target, swap, trigger).Likely an incorrect or invalid review comment.
132-156: TheonCloseremoval is not substantiated as a breaking change.The codebase search found no references to
onClosein drawer-related code. This indicates one of two scenarios:
onClosewas never part of DrawerOptionsSchema – In which case the review comment conflates a non-existent field with a breaking changeonClosewas removed and cleanly refactored – In which case there are no dangling references to verify, and the concern is already handledThe review comment claims implementations "may still reference"
onClose, but the search shows no evidence of this. Without proof of actual breaking impact (failed tests, orphaned implementations, or migration requirements), the major issue classification is premature.Secondary concern: Verify footer HTML escaping is enforced at implementation level. The schema accepts
footer: z.string().optional()but does not document that HTML content must use escaping. Per coding guidelines, implementations should validate and escape user-provided HTML content to prevent XSS.Likely an incorrect or invalid review comment.
libs/sdk/src/auth/ui/index.ts (2)
4-5: LGTM! Documentation accurately reflects Tailwind-only approach.The updated documentation correctly describes the module as using "Tailwind CSS (CDN)" and removes HTMX references, aligning with the broader migration away from HTMX.
42-42: No breaking changes detected in the export path refactor.The migration from
'./htmx-templates'to'./templates'is confirmed as complete with no remaining imports from the old path. The change is transparent to external consumers since the public API surface exported from the barrel file remains unchanged—only the internal source path has been reorganized as part of the HTMX removal refactor. No documentation updates are required.libs/sdk/src/auth/ui/templates.ts (3)
2-15: LGTM! Documentation accurately reflects the Tailwind-only approach.The header and features list have been correctly updated to remove HTMX references and emphasize Tailwind CSS with CDN usage. This aligns with the broader migration away from HTMX across the codebase.
185-249: LGTM! Incremental auth page correctly updated for Tailwind-only approach.The incremental authorization page has been properly updated to remove HTMX references from comments. The form uses standard GET submission (line 227) with proper button semantics.
560-566: LGTM! Legacy compatibility wrapper appropriately documented.The
renderToHtmlfunction documentation correctly notes that templates are already complete HTML documents, clarifying its purpose as a compatibility wrapper.
There was a problem hiding this comment.
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 (5)
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts (5)
353-367: Add missing assertions for Unknown platform behavior.This test should verify that UI is not rendered for unknown clients and optionally validate the JSON response structure, consistent with other Unknown platform tests.
🔎 Apply this diff to add missing assertions:
const result = await client.tools.call('html-card', { title: 'Test', content: 'Content', }); expect(result).toBeSuccessful(); + expect(result.hasToolUI()).toBe(false); await client.disconnect();
369-389: Add missing assertions for Unknown platform behavior.This test should verify that UI is not rendered for unknown clients, consistent with the platform's JSON-only expectations and other Unknown platform tests.
🔎 Apply this diff to add missing assertions:
results.forEach((result) => { expect(result).toBeSuccessful(); + expect(result.hasToolUI()).toBe(false); }); await client.disconnect();
458-476: Add missing UI presence verification for ChatGPT versions.This test should verify that UI rendering works consistently across different ChatGPT versions, not just that calls succeed.
🔎 Apply this diff to add missing assertions:
const result = await client.tools.call('html-card', { title: `Version ${version}`, content: 'Test', }); expect(result).toBeSuccessful(); + expect(result.hasToolUI()).toBe(true); await client.disconnect();
478-492: Add missing UI presence verification for Cursor pre-release version.This test should verify UI rendering for Cursor platform, consistent with other Cursor platform tests.
🔎 Apply this diff to add missing assertions:
const result = await client.tools.call('html-table', { headers: ['A'], rows: [['1']], }); expect(result).toBeSuccessful(); + expect(result.hasToolUI()).toBe(true); await client.disconnect();
532-548: Add platform-specific behavior verification for rapid client test.The test creates unknown clients (
RapidClient${i}) but doesn't verify the expected JSON-only behavior. Add assertions consistent with Unknown platform expectations.🔎 Apply this diff to add missing assertions:
const result = await client.tools.call('html-card', { title: `Rapid ${i}`, content: 'Content', }); expect(result).toBeSuccessful(); + expect(result.hasToolUI()).toBe(false); await client.disconnect();
♻️ Duplicate comments (4)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
585-589: Unsafe cast of structuredContent bypasses validation.This concern was raised in a previous review. The code casts
uiResult.structuredContenttoRecord<string, unknown>without validation. IfrawOutputis a primitive (string, number, null) or an array, the MCP protocol may reject it sincestructuredContentexpects an object type. ThebuildToolResponseContentfunction passesrawOutputdirectly tostructuredContentwithout enforcing object shape.Consider adding a runtime check before assignment:
Suggested fix
// Set structuredContent from UI result (contains raw tool output) - // Cast to Record<string, unknown> since MCP protocol expects object type - if (uiResult.structuredContent !== undefined && uiResult.structuredContent !== null) { - result.structuredContent = uiResult.structuredContent as Record<string, unknown>; + // MCP protocol expects structuredContent to be an object type + if ( + uiResult.structuredContent !== undefined && + uiResult.structuredContent !== null && + typeof uiResult.structuredContent === 'object' && + !Array.isArray(uiResult.structuredContent) + ) { + result.structuredContent = uiResult.structuredContent as Record<string, unknown>; }libs/ui/src/index.ts (3)
373-374: Export aliases still violate coding guidelines (duplicate issue).The export aliases using
asat lines 373-374 remain unaddressed from the previous review. These violate the coding guideline: "Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases."The previous review comment recommended:
- Rename exports at their source in
./universal/index.tstouseUniversalToolOutputanduseUniversalToolInput- Remove the aliases from this barrel export
Please address this issue as previously requested.
As per coding guidelines.
452-452: Export alias still violates coding guidelines (duplicate issue).The export alias using
asat line 452 remains unaddressed from the previous review. This violates the coding guideline: "Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases."The previous review comment recommended:
- Rename the export at its source in
./typings/index.tstoDEFAULT_TYPE_CACHE_OPTIONS- Remove the alias from this barrel export
Please address this issue as previously requested.
As per coding guidelines.
66-66: Remove export aliases from barrel exports.The following export aliases violate the guideline to export all public API surface without legacy exports or aliases:
- Line 373:
useToolOutput as useUniversalToolOutput- Line 374:
useToolInput as useUniversalToolInput- Line 452:
DEFAULT_CACHE_OPTIONS as DEFAULT_TYPE_CACHE_OPTIONSEither remove these aliases and use the original names, or re-export them from the source modules directly rather than aliasing at the barrel export level.
🧹 Nitpick comments (19)
apps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.ts (2)
22-38: Consider using test fixtures orbeforeEach/afterEachfor client lifecycle management.The client creation and disconnection pattern is repeated in every test (~20 times). If an assertion fails before
client.disconnect(), the client connection may leak. Using Playwright's fixture pattern or hooks would ensure proper cleanup and reduce duplication.🔎 Suggested approach using beforeEach/afterEach:
test.describe('Tools List Metadata', () => { let client: McpTestClient; test.beforeEach(async ({ server }) => { client = await server.createClient({ transport: 'streamable-http', clientInfo: { name: 'ChatGPT', version: '1.0.0' }, }); }); test.afterEach(async () => { await client.disconnect(); }); test('should include openai/outputTemplate for static mode tools', async () => { const tools = await client.tools.list(); const staticTool = tools.find((t) => t.name === 'static-badge'); expect(staticTool).toBeDefined(); expect(staticTool?._meta?.['openai/outputTemplate']).toBeDefined(); expect(staticTool?._meta?.['openai/outputTemplate']).toMatch(/^ui:\/\/widget\/.*\.html$/); expect(staticTool?._meta?.['openai/resultCanProduceWidget']).toBe(true); }); // ... other tests });Alternatively, if the testing framework supports custom fixtures, you could define a reusable
openaiClientfixture that handles lifecycle automatically.
250-258: Avoid unchecked type assertion; validate or use type guard.Using
as stringbypasses TypeScript's type safety. If_meta['openai/html']is not a string at runtime, subsequent operations could fail with confusing errors.🔎 Suggested safer approach:
expect(result).toBeSuccessful(); - const html = result.raw._meta?.['openai/html'] as string; - expect(html).toBeDefined(); + const html = result.raw._meta?.['openai/html']; + expect(html).toBeDefined(); + expect(typeof html).toBe('string'); expect(html).toContain('<!DOCTYPE html>'); expect(html).toContain('HTML Validation'); // Title should be in HTMLThis validates the type at runtime and provides a clearer error message if the assumption is violated.
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts (3)
393-420: Consider adding platform-specific UI assertions.While this test correctly verifies data structure consistency, it should also verify that UI-supporting platforms (ChatGPT, Claude Desktop, Cursor) return UI while Unknown platforms don't. This would make the test more comprehensive and align with the test suite's focus on platform detection.
🔎 View suggested enhancement:
for (const platform of platforms) { const client = await server.createClient({ transport: 'streamable-http', clientInfo: platform, }); const result = await client.tools.call('html-table', { headers: ['Name', 'Value'], rows: [['Test', '123']], title: `${platform.name} Test`, }); expect(result).toBeSuccessful(); const json = result.json<{ rowCount: number; columnCount: number }>(); expect(json.rowCount).toBe(1); expect(json.columnCount).toBe(2); + + // Verify platform-specific UI behavior + if (platform.name === 'Unknown') { + expect(result.hasToolUI()).toBe(false); + } else { + expect(result.hasToolUI()).toBe(true); + } await client.disconnect(); }
422-454: Add UI presence verification for concurrent cross-platform calls.This test verifies data integrity but should also confirm UI rendering behavior for each platform, consistent with the test suite's platform detection focus.
🔎 View suggested enhancement:
results.forEach((result, i) => { expect(result).toBeSuccessful(); + expect(result.hasToolUI()).toBe(true); const json = result.json<{ maxValue: number }>(); expect(json.maxValue).toBe((i + 1) * 100); });
14-550: Consider adding error scenario tests to meet 95%+ coverage requirement.The current test suite comprehensively covers happy paths but lacks error scenario testing. According to coding guidelines, tests should cover all code paths including errors.
Consider adding test cases for:
- Invalid or malformed
clientInfo(missing name/version)- Tool call failures or invalid parameters
- Network/transport errors during tool calls
- Session timeout or disconnection errors
- Invalid platform combinations
As per coding guidelines, test files should maintain 95%+ coverage across all metrics including error paths.
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts (1)
111-128: Consider wrapping disconnect in try-finally for robustness.If assertions fail before
client.disconnect(), the client connection may leak. While this is test code and not critical, using try-finally ensures cleanup even on test failures.🔎 Suggested pattern for all tests using McpTestClient:
it('should accept valid tokens and connect successfully', async () => { const token = await tokenFactory.createTestToken({ sub: 'user-123', scopes: ['read', 'write'], }); // Use McpTestClient for clean connection const client = await McpTestClient.create({ baseUrl: server.info.baseUrl, transport: 'streamable-http', auth: { token }, }).buildAndConnect(); + try { expect(client.isConnected()).toBe(true); expect(client.serverInfo.name).toBeDefined(); + } finally { await client.disconnect(); + } });libs/testing/src/client/mcp-test-client.types.ts (1)
83-116: Duplicate type definitions with platform-client-info.ts.The interfaces
McpAppsExtension,ExperimentalCapabilities,TestClientCapabilities, and constantMCP_APPS_EXTENSION_KEYare defined both here and inlibs/testing/src/platform/platform-client-info.ts. This creates maintenance overhead and potential for drift.Consider consolidating these definitions in one location and re-exporting from the other.
🔎 Suggested approach:
Define the types in
platform-client-info.ts(where they're used with platform logic) and re-export frommcp-test-client.types.ts:// In mcp-test-client.types.ts export { MCP_APPS_EXTENSION_KEY, McpAppsExtension, ExperimentalCapabilities, TestClientCapabilities, } from '../platform/platform-client-info';libs/testing/src/platform/platform-client-info.ts (1)
26-35: Duplicate TestClientInfo interface definition.
TestClientInfois defined here as a local interface, butClientInfois also defined as a type alias inmcp-test-client.types.ts. WhileClientInfoincludes more fields (fromImplementation), having two similar types could cause confusion.Consider:
- Using
ClientInfofrommcp-test-client.types.tswithPick<ClientInfo, 'name' | 'version'>if only those fields are needed- Or keeping
TestClientInfoas a minimal interface for internal platform utilities onlylibs/testing/src/client/mcp-test-client.builder.ts (1)
136-143: Consider documenting the override behavior betweenwithPlatformandwithCapabilities.When
withPlatformis called, it sets bothclientInfoandcapabilities. If a user subsequently callswithCapabilities, only the capabilities are overwritten. This ordering dependency could lead to unexpected behavior if not clearly understood.Consider adding a note in the JSDoc that calling
withPlatformafterwithCapabilitieswill overwrite the custom capabilities:/** * Note: Calling `withPlatform()` after `withCapabilities()` will overwrite * the custom capabilities. Use `withCapabilities()` after `withPlatform()` * to merge or override specific capability settings. */libs/sdk/src/tool/flows/tools-list.flow.ts (2)
24-25: Consider using a proper Zod schema forauthInfoinstead ofz.any().Per coding guidelines, avoid
anytypes without strong justification. WhileAuthInfois an external type from@modelcontextprotocol/sdk, thez.any()cast bypasses type validation entirely.Consider using
z.unknown()with a type assertion, or create a partial Zod schema that validates the fields you actually use (likesessionIdandsessionIdPayload):🔎 Suggested approach
const stateSchema = z.object({ cursor: z.string().optional(), - authInfo: z.any().optional() as z.ZodType<AuthInfo>, + authInfo: z.unknown().optional() as z.ZodType<AuthInfo | undefined>, platformType: z.string().optional() as z.ZodType<AIPlatformType | undefined>,
276-337: Platform-aware meta key generation is comprehensive but consider extracting to a helper.The logic correctly implements the platform-specific namespace requirements:
- OpenAI:
openai/*keys only- ext-apps:
ui/*keys per SEP-1865- Others:
frontmcp/*+ui/*for compatibilityHowever, this block is ~60 lines with nested conditionals. Consider extracting the meta key building logic into a separate helper function (e.g.,
buildPlatformMeta) to improve readability and testability.apps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.ts (1)
258-281: Consider adding a Gemini platform test case.Based on learnings, tests should cover behavior across platform configurations including OpenAI, Claude, Gemini, and ngrok where applicable. Gemini is mentioned as a platform that doesn't support widgets (
supportsWidgets: falsein serving-mode.ts), but there's no test validating that Gemini receives no UI meta keys.🔎 Suggested test
test.describe('Gemini Platform (No UI Support)', () => { test('should NOT have any ui meta keys for Gemini', async ({ server }) => { const client = await server.createClient({ transport: 'streamable-http', clientInfo: { name: 'gemini', version: '1.0.0' }, }); const result = await client.tools.call('html-card', { title: 'Gemini Test', content: 'Testing no-UI platform', }); expect(result).toBeSuccessful(); // Gemini should NOT receive any UI meta keys expect(result).toNotHaveMetaKey('openai/html'); expect(result).toNotHaveMetaKey('ui/html'); expect(result).toNotHaveMetaKey('frontmcp/html'); await client.disconnect(); }); });libs/sdk/src/tool/flows/call-tool.flow.ts (1)
606-621: Synchronous file I/O and dynamic require in debug block may block the event loop.The debug logging uses synchronous
fs.writeFileSyncand a dynamicrequire('fs'), which could block the event loop under heavy load. While this is debug-only code, consider:
- Using
importat the top and conditionally executing- Using async
fs.promises.writeFileto avoid blockingAlso, the
eslint-disablecomment suggests this pattern is intentional, but synchronous I/O in a flow's finalize stage could cause latency spikes.Suggested async approach
// Debug: Write full response to file if DEBUG_TOOL_RESPONSE is set if (process.env['DEBUG_TOOL_RESPONSE']) { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const fs = require('fs'); + const fs = await import('fs').then(m => m.promises); const debugOutput = { timestamp: new Date().toISOString(), tool: tool.metadata.name, rawOutput, uiResult, uiMeta, finalResult: result, }; const outputPath = process.env['DEBUG_TOOL_RESPONSE_PATH'] || '/tmp/tool-response-debug.json'; - fs.writeFileSync(outputPath, JSON.stringify(debugOutput, null, 2)); + await fs.writeFile(outputPath, JSON.stringify(debugOutput, null, 2)); console.log(`[DEBUG] Tool response written to: ${outputPath}`); }libs/ui/src/runtime/csp.ts (1)
14-19: Default CDN domains are always included, preventing restrictive policies.The
buildCSPDirectivesfunction mergesDEFAULT_CDN_DOMAINSwith user-provided domains (line 61), meaning users cannot configure a CSP that excludes these defaults. This may be intentional for convenience, but it prevents creating truly restrictive policies via theUIContentSecurityPolicyinterface.Consider adding an option to opt out of default CDNs for security-sensitive use cases:
Example approach
export interface UIContentSecurityPolicy { resourceDomains?: string[]; connectDomains?: string[]; /** If true, don't include DEFAULT_CDN_DOMAINS */ excludeDefaultCDNs?: boolean; } // In buildCSPDirectives: const allResourceDomains = csp.excludeDefaultCDNs ? validResourceDomains : [...new Set([...DEFAULT_CDN_DOMAINS, ...validResourceDomains])];libs/ui/src/adapters/__tests__/response-builder.test.ts (1)
203-205: Consider adding 'unknown' platform to test coverage.The comment on line 203-204 mentions "unknown now supports widgets with ui/html + text/html+mcp", but
'unknown'is not included innonWidgetPlatformsor thestructuredContentPlatformsarray. Consider adding a test case for the'unknown'platform to verify its expected behavior, per the coding guideline to test across platform configurations.Example test addition
it('should handle unknown platform with widget support', () => { const result = buildToolResponseContent( createOptions({ servingMode: 'inline', platformType: 'unknown', useStructuredContent: false, htmlContent: '<div>Test</div>', }), ); // Verify unknown platform behavior matches expectations expect(result.format).toBe('widget'); // or expected format });libs/ui/src/adapters/platform-meta.ts (1)
195-226: Redundant case fall-through forgeminiplatform.
geminiis listed explicitly in the switch (line 200) but falls through todefault, which then checksif (platformType === 'gemini')again (line 219). This creates redundant evaluation. Consider either:
- Giving
geminiits own case block, or- Removing
geminifrom the explicit cases since it's handled identically todefaultOption: Remove redundant explicit case
case 'claude': case 'cursor': case 'continue': case 'cody': case 'generic-mcp': - case 'gemini': default: // Other platforms use frontmcp/* namespacelibs/testing/src/ui/ui-assertions.ts (2)
329-351: Consider adding a usage caveat to the JSDoc.The
assertNoMixedNamespacesmethod strictly enforces a single namespace, which may surprise users testing FrontMCP platforms that legitimately use bothfrontmcp/*andui/*keys. The JSDoc mentions the expected namespace but could benefit from a note directing users toassertPlatformMetafor multi-namespace platforms.🔎 Suggested JSDoc enhancement:
/** * Assert that no cross-namespace pollution exists in meta. * @param result - The tool result wrapper * @param expectedNamespace - The namespace that SHOULD be present * @throws Error if other namespaces are found + * + * @note For platforms like Claude that use both frontmcp/* and ui/* keys, + * use `assertPlatformMeta()` instead, which handles multi-namespace platforms. */
367-388: Consider extracting platform-specific key mapping to reduce duplication.The switch statements for determining
mimeTypeKeyandhtmlKeybased on platform are duplicated between this file andui-matchers.ts(lines 493-504 and 537-548). Consider adding helper functions likegetPlatformMimeTypeKey(platform)andgetPlatformHtmlKey(platform)toplatform-types.tsto centralize this logic.🔎 Example helper functions for platform-types.ts:
/** * Get the meta key for MIME type based on platform. */ export function getPlatformMimeTypeKey(platform: TestPlatformType): string { switch (platform) { case 'openai': return 'openai/mimeType'; case 'ext-apps': return 'ui/mimeType'; default: return 'frontmcp/mimeType'; } } /** * Get the meta key for HTML based on platform. */ export function getPlatformHtmlKey(platform: TestPlatformType): string { switch (platform) { case 'openai': return 'openai/html'; case 'ext-apps': return 'ui/html'; default: return 'frontmcp/html'; } }Also applies to: 404-427
libs/testing/src/platform/platform-types.ts (1)
97-122: Optional: Consider consolidating identical prefix functions or documenting future divergence.
getToolsListMetaPrefixesandgetToolCallMetaPrefixescurrently have identical implementations. If they're expected to diverge in the future (e.g., different keys for list vs. call responses), consider adding a brief comment explaining this. Otherwise, they could share a private helper to reduce duplication.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.ts(4 hunks)apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts(8 hunks)apps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.ts(6 hunks)apps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.ts(1 hunks)libs/sdk/src/flows/flow.instance.ts(1 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(7 hunks)libs/sdk/src/tool/flows/tools-list.flow.ts(7 hunks)libs/testing/src/client/mcp-test-client.builder.ts(2 hunks)libs/testing/src/client/mcp-test-client.ts(6 hunks)libs/testing/src/client/mcp-test-client.types.ts(3 hunks)libs/testing/src/example-tools/index.ts(1 hunks)libs/testing/src/example-tools/tool-configs.ts(1 hunks)libs/testing/src/fixtures/fixture-types.ts(2 hunks)libs/testing/src/fixtures/test-fixture.ts(6 hunks)libs/testing/src/index.ts(1 hunks)libs/testing/src/platform/index.ts(1 hunks)libs/testing/src/platform/platform-client-info.ts(1 hunks)libs/testing/src/platform/platform-types.ts(1 hunks)libs/testing/src/ui/ui-assertions.ts(2 hunks)libs/testing/src/ui/ui-matchers.ts(4 hunks)libs/ui/src/adapters/__tests__/platform-meta.test.ts(2 hunks)libs/ui/src/adapters/__tests__/response-builder.test.ts(7 hunks)libs/ui/src/adapters/__tests__/serving-mode.test.ts(5 hunks)libs/ui/src/adapters/platform-meta.ts(1 hunks)libs/ui/src/adapters/serving-mode.ts(10 hunks)libs/ui/src/index.ts(3 hunks)libs/ui/src/runtime/csp.ts(2 hunks)libs/ui/src/runtime/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/ui/src/adapters/tests/serving-mode.test.ts
- libs/testing/src/client/mcp-test-client.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/sdk/src/flows/flow.instance.tslibs/ui/src/runtime/index.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/__tests__/platform-meta.test.tsapps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.tslibs/testing/src/platform/index.tsapps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.tslibs/testing/src/platform/platform-types.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/client/mcp-test-client.builder.tslibs/testing/src/index.tslibs/testing/src/platform/platform-client-info.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/fixtures/fixture-types.tslibs/ui/src/runtime/csp.tslibs/testing/src/fixtures/test-fixture.tslibs/ui/src/adapters/platform-meta.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/testing/src/example-tools/index.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/testing/src/client/mcp-test-client.types.tslibs/testing/src/example-tools/tool-configs.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tslibs/ui/src/index.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/ui/src/adapters/__tests__/platform-meta.test.tsapps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tslibs/ui/src/adapters/__tests__/response-builder.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/tool/flows/tools-list.flow.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/flows/flow.instance.tslibs/ui/src/runtime/index.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/__tests__/platform-meta.test.tslibs/testing/src/platform/index.tslibs/testing/src/platform/platform-types.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/client/mcp-test-client.builder.tslibs/testing/src/index.tslibs/testing/src/platform/platform-client-info.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/fixtures/fixture-types.tslibs/ui/src/runtime/csp.tslibs/testing/src/fixtures/test-fixture.tslibs/ui/src/adapters/platform-meta.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/testing/src/example-tools/index.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/testing/src/client/mcp-test-client.types.tslibs/testing/src/example-tools/tool-configs.tslibs/ui/src/index.ts
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/runtime/index.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/__tests__/platform-meta.test.tslibs/ui/src/runtime/csp.tslibs/ui/src/adapters/platform-meta.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/ui/src/index.ts
libs/ui/src/**/*.test.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.test.ts: Maintain 95%+ test coverage across statements, branches, functions, and lines
Test HTML escaping for user-provided content to prevent XSS attacks
Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Files:
libs/ui/src/adapters/__tests__/platform-meta.test.tslibs/ui/src/adapters/__tests__/response-builder.test.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Files:
libs/testing/src/index.tslibs/ui/src/index.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/__tests__/platform-meta.test.tsapps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.tslibs/testing/src/platform/index.tsapps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.tslibs/testing/src/platform/platform-types.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/client/mcp-test-client.builder.tslibs/testing/src/index.tslibs/testing/src/platform/platform-client-info.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/fixtures/test-fixture.tslibs/ui/src/adapters/platform-meta.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/testing/src/example-tools/index.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/testing/src/client/mcp-test-client.types.tslibs/testing/src/example-tools/tool-configs.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/ui/src/adapters/__tests__/platform-meta.test.tsapps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/example-tools/tool-configs.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/ui/src/adapters/__tests__/platform-meta.test.tsapps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/ui/src/adapters/__tests__/response-builder.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-ui/e2e/platform-detection.e2e.test.tslibs/ui/src/adapters/__tests__/platform-meta.test.tsapps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/index.tslibs/testing/src/ui/ui-assertions.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/testing/src/example-tools/tool-configs.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Applied to files:
libs/ui/src/runtime/index.tslibs/ui/src/runtime/csp.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/ui/src/runtime/index.tslibs/testing/src/platform/index.tslibs/testing/src/index.tslibs/testing/src/example-tools/index.tslibs/testing/src/example-tools/tool-configs.tslibs/ui/src/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `getCapabilities()` for dynamic capability exposure instead of hardcoding capabilities in adapters
Applied to files:
libs/ui/src/adapters/serving-mode.tslibs/ui/src/adapters/__tests__/platform-meta.test.tslibs/testing/src/platform/index.tslibs/testing/src/platform/platform-types.tslibs/testing/src/client/mcp-test-client.builder.tslibs/testing/src/index.tslibs/testing/src/platform/platform-client-info.tslibs/ui/src/adapters/platform-meta.tslibs/ui/src/adapters/__tests__/response-builder.test.tslibs/testing/src/client/mcp-test-client.types.tslibs/testing/src/example-tools/tool-configs.tslibs/ui/src/index.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
Applied to files:
libs/ui/src/adapters/serving-mode.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/adapters/__tests__/platform-meta.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/common/records/**/*.ts : Centralize record types in common/records and import from there instead of from module-specific files
Applied to files:
libs/testing/src/platform/index.tslibs/testing/src/platform/platform-types.tslibs/testing/src/example-tools/index.tslibs/testing/src/example-tools/tool-configs.tslibs/ui/src/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/testing/src/platform/platform-types.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/fixtures/fixture-types.tslibs/testing/src/fixtures/test-fixture.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/testing/src/client/mcp-test-client.types.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/testing/src/platform/platform-types.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/platform/platform-client-info.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/fixtures/fixture-types.tslibs/testing/src/client/mcp-test-client.types.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/tool/flows/tools-list.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
libs/testing/src/fixtures/test-fixture.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
libs/ui/src/adapters/__tests__/response-builder.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
libs/testing/src/example-tools/index.tslibs/testing/src/example-tools/tool-configs.tslibs/ui/src/index.ts
📚 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:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/ui/src/index.ts
🧬 Code graph analysis (12)
libs/sdk/src/flows/flow.instance.ts (1)
libs/sdk/src/errors/mcp.error.ts (1)
InternalMcpError(129-142)
libs/ui/src/adapters/__tests__/platform-meta.test.ts (1)
libs/ui/src/adapters/platform-meta.ts (1)
buildUIMeta(166-228)
apps/e2e/demo-e2e-ui/e2e/openai-resource-flow.e2e.test.ts (1)
libs/testing/src/http-mock/http-mock.ts (1)
html(554-560)
apps/e2e/demo-e2e-ui/e2e/platform-meta-keys.e2e.test.ts (1)
libs/testing/src/ui/ui-assertions.ts (1)
UIAssertions(52-446)
libs/testing/src/platform/platform-types.ts (3)
libs/testing/src/index.ts (10)
TestPlatformType(221-221)PlatformMetaNamespace(221-221)getPlatformMetaNamespace(224-224)getPlatformMimeType(225-225)isOpenAIPlatform(226-226)isExtAppsPlatform(227-227)isFrontmcpPlatform(228-228)getToolsListMetaPrefixes(229-229)getToolCallMetaPrefixes(230-230)getForbiddenMetaPrefixes(231-231)libs/testing/src/platform/index.ts (10)
TestPlatformType(27-27)PlatformMetaNamespace(27-27)getPlatformMetaNamespace(31-31)getPlatformMimeType(32-32)isOpenAIPlatform(33-33)isExtAppsPlatform(34-34)isFrontmcpPlatform(35-35)getToolsListMetaPrefixes(36-36)getToolCallMetaPrefixes(37-37)getForbiddenMetaPrefixes(38-38)libs/sdk/src/common/interfaces/tool.interface.ts (1)
platform(120-133)
libs/sdk/src/tool/flows/call-tool.flow.ts (3)
libs/ui/src/registry/tool-ui.registry.ts (1)
isUIRenderFailure(91-93)libs/ui/src/registry/index.ts (1)
isUIRenderFailure(45-45)libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)
libs/testing/src/client/mcp-test-client.builder.ts (2)
libs/testing/src/platform/platform-types.ts (1)
TestPlatformType(29-38)libs/testing/src/platform/platform-client-info.ts (3)
getPlatformClientInfo(44-93)getPlatformCapabilities(171-189)TestClientCapabilities(157-160)
libs/testing/src/platform/platform-client-info.ts (2)
libs/sdk/src/common/interfaces/tool.interface.ts (2)
platform(120-133)clientInfo(149-155)libs/testing/src/platform/platform-types.ts (1)
TestPlatformType(29-38)
libs/ui/src/adapters/__tests__/response-builder.test.ts (2)
libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)libs/ui/src/adapters/platform-meta.ts (1)
AIPlatformType(24-33)
libs/testing/src/client/mcp-test-client.types.ts (1)
libs/testing/src/platform/platform-types.ts (1)
TestPlatformType(29-38)
libs/testing/src/example-tools/tool-configs.ts (2)
libs/testing/src/example-tools/index.ts (14)
basicUIToolInputSchema(22-22)basicUIToolOutputSchema(23-23)BASIC_UI_TOOL_CONFIG(20-20)fullUIToolInputSchema(24-24)fullUIToolOutputSchema(25-25)FULL_UI_TOOL_CONFIG(21-21)generateBasicUIToolOutput(29-29)generateFullUIToolOutput(29-29)EXPECTED_OPENAI_TOOLS_LIST_META_KEYS(33-33)EXPECTED_OPENAI_TOOL_CALL_META_KEYS(34-34)EXPECTED_EXTAPPS_TOOLS_LIST_META_KEYS(35-35)EXPECTED_EXTAPPS_TOOL_CALL_META_KEYS(36-36)EXPECTED_FRONTMCP_TOOLS_LIST_META_KEYS(37-37)EXPECTED_FRONTMCP_TOOL_CALL_META_KEYS(38-38)libs/testing/src/index.ts (14)
basicUIToolInputSchema(246-246)basicUIToolOutputSchema(247-247)BASIC_UI_TOOL_CONFIG(244-244)fullUIToolInputSchema(248-248)fullUIToolOutputSchema(249-249)FULL_UI_TOOL_CONFIG(245-245)generateBasicUIToolOutput(251-251)generateFullUIToolOutput(252-252)EXPECTED_OPENAI_TOOLS_LIST_META_KEYS(254-254)EXPECTED_OPENAI_TOOL_CALL_META_KEYS(255-255)EXPECTED_EXTAPPS_TOOLS_LIST_META_KEYS(256-256)EXPECTED_EXTAPPS_TOOL_CALL_META_KEYS(257-257)EXPECTED_FRONTMCP_TOOLS_LIST_META_KEYS(258-258)EXPECTED_FRONTMCP_TOOL_CALL_META_KEYS(259-259)
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts (2)
libs/testing/src/client/mcp-test-client.ts (1)
McpTestClient(63-1078)libs/testing/src/index.ts (1)
McpTestClient(46-46)
⏰ 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
…ion with content security options
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/e2e/demo-e2e-multiapp/project.json (1)
28-35: The "exec node" concern from previous review has been resolved.The previous review flagged the use of
exec nodeas Unix-only. The current code correctly uses plainnodewithout theexecprefix, making it cross-platform compatible.apps/e2e/demo-e2e-serverless/project.json (1)
28-35: The "exec" prefix issue raised in previous review has been addressed.The past review correctly identified that
exec nodewouldn't work on Windows. The current implementation uses plainnode, which is cross-platform compatible.libs/sdk/src/tool/flows/call-tool.flow.ts (1)
585-589: Validate structuredContent against output schema instead of unsafe cast.The code assigns
uiResult.structuredContenttoresult.structuredContentwith an unsafe cast toRecord<string, unknown>. This bypasses runtime validation—if the value is a primitive or array, it may fail MCP protocol validation despite the type assertion. The tool's output schema has already validated the content at line 570; consider preserving that validated result or explicitly validatinguiResult.structuredContentbefore assignment.libs/ui/src/components/modal.ts (1)
216-218: Document security constraints forconfirmHref.The JSDoc for
confirmHreflacks security documentation. Developers need to understand which protocols are allowed and what happens when validation fails.🔎 Add comprehensive security documentation
- /** Confirm action URL */ + /** + * Confirm action URL + * @remarks Only safe protocols are allowed: http://, https://, /, #, mailto:, tel: + * Unsafe protocols (javascript:, data:, vbscript:, etc.) will be rejected and the + * confirm button will fall back to a dismissal button. Never pass user-controlled + * or unvalidated URLs without proper sanitization. + * @example confirmHref: '/api/confirm' + * @example confirmHref: 'https://example.com/action' + */ confirmHref?: string;As per past review comments and coding guidelines requiring JSDoc examples.
🧹 Nitpick comments (5)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
606-623: Use cross-platform temporary directory for debug output.The default path
/tmp/tool-response-debug.jsonis Unix-specific and will fail on Windows. Use Node'sos.tmpdir()for cross-platform compatibility.🔎 Proposed fix for cross-platform compatibility
+ const os = require('os'); const fs = require('fs').promises; const debugOutput = { timestamp: new Date().toISOString(), tool: tool.metadata.name, rawOutput, uiResult, uiMeta, finalResult: result, }; - const outputPath = process.env['DEBUG_TOOL_RESPONSE_PATH'] || '/tmp/tool-response-debug.json'; + const outputPath = process.env['DEBUG_TOOL_RESPONSE_PATH'] || + `${os.tmpdir()}/tool-response-debug.json`; // Use async write to avoid blocking the event loop (fire-and-forget)libs/sdk/src/flows/flow.instance.ts (1)
406-423: Excellent fix for the finalize-stage error masking issue! Consider using structured logging for consistency.The implementation correctly addresses the critical issue flagged in the previous review by introducing the
suppressErrorsoption. The error handling logic properly prevents finalize errors from masking original errors when suppression is enabled, while still throwing finalize errors in the normal flow when appropriate.However, the logging pattern at lines 414-417 should follow the structured logging approach used at lines 115-118 to avoid the Node.js util.inspect bug with Zod errors (mentioned in the comment at lines 113-114). This ensures consistency and defensive coding across the codebase.
🔎 Suggested refactor for consistent structured logging
if (res.outcome === 'unknown_error' || res.outcome === 'fail') { const finalizeError = res.control ?? new InternalMcpError('Finalize stage failed'); if (options?.suppressErrors) { - // Log finalizes errors but doesn't throw when called from finally blocks + // Log finalize errors but don't throw when called from finally blocks // This prevents masking the original error - this.scope.logger.error( - '[FrontMCP] Finalize stage error (suppressed to preserve original error):', - finalizeError, - ); + console.error('[FlowInstance] Finalize stage error (suppressed to preserve original error):', { + name: finalizeError instanceof Error ? finalizeError.name : 'UnknownError', + message: finalizeError instanceof Error ? finalizeError.message : String(finalizeError), + }); return res; } throw finalizeError; }libs/sdk/src/tool/flows/tools-list.flow.ts (1)
102-117: Add type guard for safe access to scope.notifications.The code casts
this.scopetoScopeand directly accessesscope.notifications?.getPlatformType(sessionId)without validating that these methods exist. While the optional chaining (?.) provides some protection, a more robust type guard would prevent potential runtime issues.Consider adding a type guard similar to the one used later in
parseTools(lines 245-260) to verify the scope has the expected structure before accessingnotifications.getPlatformType.🔎 Suggested type guard
- // Cast scope to access notifications service for platform detection - const scope = this.scope as Scope; + // Type guard for scope with notifications service + const hasNotifications = (obj: unknown): obj is Scope & { + notifications: { getPlatformType: (sessionId: string) => AIPlatformType | undefined }; + } => { + return ( + obj !== null && + typeof obj === 'object' && + 'notifications' in obj && + typeof (obj as any).notifications === 'object' && + typeof (obj as any).notifications?.getPlatformType === 'function' + ); + }; // Get platform type: first check sessionIdPayload (detected from user-agent), // then fall back to notification service (detected from MCP clientInfo), // finally default to 'unknown' const platformType: AIPlatformType = authInfo?.sessionIdPayload?.platformType ?? - (sessionId ? scope.notifications?.getPlatformType(sessionId) : undefined) ?? + (sessionId && hasNotifications(this.scope) ? this.scope.notifications.getPlatformType(sessionId) : undefined) ?? 'unknown';libs/ui/src/universal/runtime-builder.ts (1)
457-466: Basic minification may break code with regex or string edge cases.The minification logic removes comments using simple regex patterns (lines 461-462) that could incorrectly match comment-like patterns in strings or regex literals. However, since this is only applied to controlled, generated code (not user input), the risk is low.
For improved robustness, consider using a proper JavaScript minifier like
terseroresbuildwhen minification is enabled.libs/ui/src/types/ui-config.ts (1)
289-334: Well-documented security field with practical examples.The
contentSecurityfield addition includes excellent documentation with:
- Security context explanation
- Multiple usage examples (default, inline scripts, bypass)
- Clear best practice guidance
Minor observation: Some security context is duplicated from the
UIContentSecurityinterface documentation (lines 37-70). Consider consolidating to reduce duplication, though the current approach does provide helpful context in both locations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/e2e/demo-e2e-cache/project.json(1 hunks)apps/e2e/demo-e2e-codecall/project.json(1 hunks)apps/e2e/demo-e2e-errors/project.json(1 hunks)apps/e2e/demo-e2e-hooks/project.json(1 hunks)apps/e2e/demo-e2e-multiapp/project.json(1 hunks)apps/e2e/demo-e2e-notifications/project.json(1 hunks)apps/e2e/demo-e2e-openapi/project.json(1 hunks)apps/e2e/demo-e2e-orchestrated/project.json(1 hunks)apps/e2e/demo-e2e-providers/project.json(1 hunks)apps/e2e/demo-e2e-public/project.json(1 hunks)apps/e2e/demo-e2e-redis/project.json(1 hunks)apps/e2e/demo-e2e-serverless/project.json(1 hunks)apps/e2e/demo-e2e-transparent/project.json(1 hunks)apps/e2e/demo-e2e-ui/project.json(1 hunks)libs/sdk/src/flows/flow.instance.ts(5 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(7 hunks)libs/sdk/src/tool/flows/tools-list.flow.ts(7 hunks)libs/ui/src/components/modal.ts(5 hunks)libs/ui/src/types/index.ts(1 hunks)libs/ui/src/types/ui-config.ts(2 hunks)libs/ui/src/universal/cached-runtime.ts(1 hunks)libs/ui/src/universal/index.ts(1 hunks)libs/ui/src/universal/runtime-builder.ts(1 hunks)libs/ui/src/universal/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/e2e/demo-e2e-openapi/project.json
- apps/e2e/demo-e2e-codecall/project.json
- apps/e2e/demo-e2e-orchestrated/project.json
- apps/e2e/demo-e2e-providers/project.json
- libs/ui/src/universal/index.ts
- apps/e2e/demo-e2e-cache/project.json
🧰 Additional context used
📓 Path-based instructions (6)
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/types/index.tslibs/ui/src/types/ui-config.tslibs/ui/src/universal/runtime-builder.tslibs/ui/src/components/modal.tslibs/ui/src/universal/types.tslibs/ui/src/universal/cached-runtime.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/ui/src/types/index.tslibs/ui/src/types/ui-config.tslibs/ui/src/universal/runtime-builder.tslibs/ui/src/components/modal.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/universal/types.tslibs/ui/src/universal/cached-runtime.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/ui/src/types/index.tslibs/ui/src/types/ui-config.tslibs/ui/src/universal/runtime-builder.tslibs/ui/src/components/modal.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/universal/types.tslibs/ui/src/universal/cached-runtime.ts
libs/ui/src/components/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/components/**/*.ts: Validate component options usingvalidateOptions()helper and return error box on validation failure
Add JSDoc examples with @example tags showing basic usage and options patterns for all components
Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Files:
libs/ui/src/components/modal.ts
libs/ui/src/components/**
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Organize components into schema.ts (definitions) and implementation .ts files with matching names
Files:
libs/ui/src/components/modal.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/flows/call-tool.flow.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
apps/e2e/demo-e2e-redis/project.jsonapps/e2e/demo-e2e-errors/project.jsonapps/e2e/demo-e2e-hooks/project.jsonapps/e2e/demo-e2e-public/project.jsonapps/e2e/demo-e2e-multiapp/project.jsonapps/e2e/demo-e2e-serverless/project.jsonapps/e2e/demo-e2e-transparent/project.jsonapps/e2e/demo-e2e-notifications/project.jsonapps/e2e/demo-e2e-ui/project.json
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
libs/ui/src/types/index.tslibs/ui/src/types/ui-config.tslibs/ui/src/universal/runtime-builder.tslibs/ui/src/components/modal.tslibs/ui/src/universal/cached-runtime.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
libs/ui/src/types/index.tslibs/ui/src/types/ui-config.tslibs/ui/src/universal/runtime-builder.tslibs/ui/src/components/modal.tslibs/ui/src/universal/cached-runtime.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-multiapp/project.jsonlibs/sdk/src/tool/flows/tools-list.flow.tslibs/ui/src/universal/types.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
libs/ui/src/components/modal.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/ui/src/components/modal.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/common/records/**/*.ts : Centralize record types in common/records and import from there instead of from module-specific files
Applied to files:
libs/ui/src/universal/types.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
libs/ui/src/universal/types.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/ui/src/universal/types.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/ui/src/universal/types.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/universal/types.tslibs/ui/src/universal/cached-runtime.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Applied to files:
libs/ui/src/universal/cached-runtime.ts
🧬 Code graph analysis (7)
libs/ui/src/types/ui-config.ts (1)
libs/ui/src/types/index.ts (1)
UIContentSecurity(19-19)
libs/ui/src/universal/runtime-builder.ts (1)
libs/ui/src/universal/types.ts (3)
UniversalRuntimeOptions(222-240)UNIVERSAL_CDN(263-271)UniversalRuntimeResult(245-254)
libs/ui/src/components/modal.ts (3)
libs/sdk/src/auth/ui/base-layout.ts (1)
escapeHtml(165-173)libs/sdk/src/auth/ui/templates.ts (1)
escapeHtml(87-87)libs/ui/src/layouts/base.ts (1)
escapeHtml(146-146)
libs/sdk/src/flows/flow.instance.ts (1)
libs/sdk/src/errors/mcp.error.ts (1)
InternalMcpError(129-142)
libs/sdk/src/tool/flows/tools-list.flow.ts (9)
libs/ui/src/adapters/platform-meta.ts (1)
AIPlatformType(24-33)libs/ui/src/index.ts (1)
AIPlatformType(51-51)libs/sdk/src/notification/notification.service.ts (1)
AIPlatformType(19-19)libs/sdk/src/common/types/auth/session.types.ts (1)
AIPlatformType(13-22)libs/sdk/src/notification/index.ts (1)
AIPlatformType(12-12)libs/sdk/src/tool/ui/platform-adapters.ts (1)
AIPlatformType(13-13)libs/testing/src/client/mcp-test-client.ts (1)
sessionId(207-209)libs/testing/src/index.ts (1)
isOpenAIPlatform(226-226)libs/testing/src/platform/index.ts (1)
isOpenAIPlatform(33-33)
libs/sdk/src/tool/flows/call-tool.flow.ts (2)
libs/ui/src/registry/tool-ui.registry.ts (1)
isUIRenderFailure(91-93)libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)
libs/ui/src/universal/cached-runtime.ts (5)
libs/ui/src/universal/index.ts (7)
RUNTIME_PLACEHOLDERS(154-154)CachedRuntimeOptions(156-156)CDNType(62-62)ContentSecurityOptions(65-65)UNIVERSAL_CDN(68-68)CachedRuntimeResult(157-157)buildComponentCode(152-152)libs/ui/src/index.ts (6)
RUNTIME_PLACEHOLDERS(413-413)CachedRuntimeOptions(414-414)CDNType(364-364)UNIVERSAL_CDN(418-418)CachedRuntimeResult(415-415)buildComponentCode(412-412)libs/ui/src/universal/types.ts (3)
CDNType(194-194)ContentSecurityOptions(199-217)UNIVERSAL_CDN(263-271)libs/ui/src/bundler/cache.ts (1)
keys(253-255)libs/ui/src/registry/tool-ui.registry.ts (1)
buildComponentCode(635-662)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (23)
apps/e2e/demo-e2e-errors/project.json (1)
29-34: LGTM! Improved serve target configuration.The migration from
@nx/js:nodeexecutor to explicitnx:run-commandswithdependsOn: ["build"]is a good change that:
- Makes the execution flow more transparent and explicit
- Ensures the build artifact exists before attempting to serve
- Maintains path consistency with the build target's outputPath (line 15)
- Aligns with the standardization effort across E2E demo projects
Based on learnings, this continues to use the Nx build system appropriately.
apps/e2e/demo-e2e-notifications/project.json (1)
28-35: LGTM! Appropriate migration for e2e testing.The switch from
@nx/js:nodetonx:run-commandswith explicit post-build execution is well-suited for an e2e test application. ThedependsOn: ["build"]ensures correct execution order, and the explicit command path aligns with the build output configuration. While this removes watch mode capabilities, that's not typically needed for e2e tests and makes the execution flow more transparent and debuggable.apps/e2e/demo-e2e-transparent/project.json (1)
28-35: LGTM—consistent E2E serve refactoring across all projects.The migration from
@nx/js:nodetonx:run-commandswith explicit build dependency and post-build command execution is implemented consistently across all 6 E2E projects. The approach is sound:
- ✅
dependsOn: ["build"]ensures the artifact exists before serving- ✅
cwd: "{workspaceRoot}"provides consistent path resolution- ✅ Plain
nodecommand is cross-platform compatible (Windows, macOS, Linux)This pattern aligns with the PR objective of "adding E2E test resilience" by making the execution flow explicit and controllable.
libs/sdk/src/tool/flows/call-tool.flow.ts (3)
464-484: LGTM: Graceful degradation for UI rendering failures.The graceful degradation path properly handles UI rendering failures by logging a warning and proceeding without UI content. This ensures tool execution succeeds even when UI rendering fails, which aligns with the PR objective of enhancing test resilience.
556-567: Improved error handling: skip finalization on early failures.The change from throwing errors to returning early is appropriate since the
finalizestage runs even during cleanup after execution failures. This prevents masking the original error with a generic finalization error. The verbose logging provides debugging visibility while relying on the flow framework to propagate the original error.
545-550: LGTM: Improved documentation for finalize stage behavior.The updated documentation clearly explains that the
finalizestage runs during cleanup even after execution failures, and that error propagation is handled by the flow framework. This helps maintainers understand the error handling pattern.apps/e2e/demo-e2e-redis/project.json (1)
29-34: Serve target configuration is correct and consistently applied across all e2e demo projects.The configuration properly uses
nx:run-commandswith explicitdependsOn: ["build"]to ensure compilation completes before execution. The command path correctly references the webpack build output, and the workspace root cwd is appropriate for this use case. This pattern is standardized across all 14 e2e demo projects in the repository.libs/ui/src/components/modal.ts (1)
271-286: Security implementation looks good.The confirm button implementation correctly:
- Validates
confirmHrefusingisSafeUrlbefore rendering a link- Falls back to a safe dismissal button when the URL is unsafe or missing
- Applies
escapeHtmlfor defense-in-depthThis addresses the critical XSS vulnerability flagged in previous reviews.
Note: Once the warning is added to
isSafeUrl(per earlier comment), rejected URLs will be properly logged for debugging.libs/sdk/src/flows/flow.instance.ts (1)
434-434: LGTM! Call sites correctly implement the error suppression pattern.All invocations of
runFinalizeStage()follow the correct pattern:
- Lines 434, 452, 474, 493: Suppression in
finallyblocks prevents finalize errors from masking the original error being thrown from the try block.- Lines 439, 457, 478, 498: Suppression before throwing
FlowControlensures the intended control flow signal is not overridden by a finalize error.- Lines 445, 507: No suppression in the happy path allows finalize errors to be properly thrown when no other error has occurred.
This implementation ensures robust error propagation throughout the flow lifecycle.
Also applies to: 439-439, 445-445, 452-452, 457-457, 474-474, 478-478, 493-493, 498-498, 507-507
libs/ui/src/types/index.ts (1)
18-19: LGTM! Clean type export for XSS protection configuration.The addition of the
UIContentSecurityexport alongsideUIContentSecurityPolicyprovides a clear separation between CSP directives (external resource policies) and XSS protection settings (sanitization behavior). The comment accurately describes its purpose.libs/sdk/src/tool/flows/tools-list.flow.ts (2)
22-27: Documented exception to strict typing for external SDK type.The use of
z.any()forauthInfois appropriately justified with an inline comment explaining thatAuthInfois an external type from@modelcontextprotocol/sdkthat varies by SDK version. This is a reasonable exception to the strict typing rule when dealing with third-party types that may have version variance.
234-341: Excellent platform-aware meta generation with robust type guard.The platform-specific meta generation is well-implemented with clear separation:
- OpenAI:
openai/*keys only (proprietary ChatGPT format)- ext-apps:
ui/*keys only (SEP-1865 MCP Apps spec)- Other platforms:
frontmcp/*+ui/*for compatibilityThe type guard at lines 245-260 is exemplary—it thoroughly validates that
toolUIexists with the required methods before accessing them. This prevents runtime errors and provides clear error messages.libs/ui/src/universal/runtime-builder.ts (3)
93-160: URL validation properly prevents javascript: injection.The markdown parser now includes proper URL scheme validation via
isSafeUrl()(lines 106-117) that blocks dangerous schemes likejavascript:,data:, andvbscript:by default. Links are validated before rendering (lines 136-139), and malicious links are rendered as plain text rather than clickable links.The HTML escaping at line 122 prevents basic XSS via HTML tag injection. This addresses the XSS vulnerabilities noted in past reviews.
✅ Addresses past review concerns about javascript: URL injection.
169-302: HTML renderer properly sanitizes content by default.The HTML renderer now includes XSS protection that removes
<script>tags (line 191) and event handlers (line 192) before rendering withdangerouslySetInnerHTML. Sanitization is only bypassed whenallowInlineScriptsorbypassSanitizationis explicitly enabled in the content security options.This provides defense-in-depth protection and addresses the XSS concerns from past reviews.
✅ Addresses past review concerns about HTML renderer XSS vulnerability.
201-225: Markdown renderer fallback uses sanitized parser.The markdown renderer fallback (line 219) now uses
window.__frontmcp.parseMarkdown, which includes proper URL validation and HTML escaping (as reviewed above in lines 93-160). This resolves the XSS vulnerability concern from past reviews.✅ Addresses past review concerns about markdown renderer XSS vulnerability.
libs/ui/src/types/ui-config.ts (1)
37-124: Excellent security-focused interface with comprehensive documentation.The
UIContentSecurityinterface is exceptionally well-documented with:
- Clear explanation of the platform isolation context (double-iframe sandbox)
- Detailed security implications for each option
- Default-secure approach (all protections enabled by default)
- Appropriate warnings about when to disable protections
This sets a high bar for API documentation and security best practices.
libs/ui/src/universal/types.ts (3)
26-185: Justified use ofanyfor React component types.The use of
anyin React component types (React.ComponentType<any>) at lines 32, 39, 122, 163, and 170 is appropriately justified. React components require flexible prop types, and usingunknownwould break component composition. All instances include eslint-disable comments, indicating deliberate architectural decisions.
187-271: Well-structured runtime configuration types with clear documentation.The runtime types (
UniversalRuntimeOptions,UniversalRuntimeResult,ContentSecurityOptions) are clearly defined with appropriate documentation. TheUNIVERSAL_CDNconstant provides centralized CDN URL management, and the dual CDN type support (ESM/UMD) enables both network-open and network-blocked scenarios.
277-332: Comprehensive content type detection with clear priority order.The
detectContentTypefunction implements a well-thought-out detection strategy with clear priority:
- Function components → 'react'
- Module syntax + JSX → 'react' (needs transpilation)
- JSX + Markdown (no modules) → 'mdx'
- Pure markdown → 'markdown'
- Default → 'html'
The logic handles edge cases well and includes helpful comments explaining each detection path. The heuristics cover the common patterns while avoiding false positives.
libs/ui/src/universal/cached-runtime.ts (4)
234-296: Markdown parser includes proper URL validation matching runtime-builder.ts.The inline markdown parser implementation includes the same URL validation logic as
runtime-builder.ts, withisSafeUrl()blocking dangerous schemes by default. HTML escaping (line 264) and link validation (lines 276-278) provide defense-in-depth protection.✅ Consistent with fixes in runtime-builder.ts that address past XSS concerns.
298-428: HTML renderer properly sanitizes content by default.The HTML renderer implementation (lines 316-333) matches the sanitization logic in
runtime-builder.ts, removing<script>tags (line 325) and event handlers (line 326) unless explicitly bypassed. This provides consistent XSS protection across the codebase.
673-746: Cache implementation with proper TTL and eviction.The cache implementation includes:
- Comprehensive cache key generation based on all options (lines 83-104)
- TTL expiration checking (line 681)
- FIFO eviction when max entries reached (lines 728-734)
The cache logic is sound and will properly manage memory usage with the configurable limits.
822-871: Data injection safely handles serialization errors.The
buildDataInjectionCodefunction includes proper error handling for JSON serialization via thesafeJsonhelper (lines 834-840), which catches serialization errors and falls back to'null'. This prevents the builder from crashing on circular references or other unserializable data.
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 (1)
libs/ui/src/components/modal.ts (1)
102-186: Add option validation and documentation examples tomodalandconfirmModalfunctions.The
modalandconfirmModalfunctions bypass option validation required by coding guidelines. Following the pattern established inbutton.ts, createmodal.schema.tswith Zod schemas, then validate options usingvalidateOptions()at the start of each function and return the error box on validation failure. Additionally, add@exampleJSDoc tags to both functions demonstrating basic usage and option patterns.
♻️ Duplicate comments (5)
libs/ui/src/universal/renderers/html.renderer.ts (2)
27-49: XSS risk remains:htmlRendererstill uses unsanitizeddangerouslySetInnerHTML.This issue was previously flagged but remains unresolved. The
htmlRendererrenders raw HTML without any sanitization, creating an XSS vulnerability if the source contains user-controlled content.As per coding guidelines, user-provided content should use the
escapeHtml()utility. Consider either:
- Using
safeHtmlRendereras the default export, or- Clearly documenting that
htmlRendereris for trusted content only
58-70: Regex-basedsanitizeHtml()remains inadequate for production use.This issue was previously flagged but remains unresolved. The regex-based approach has known limitations that allow XSS vectors to bypass:
- HTML entity-encoded payloads (e.g.,
<script>)javascript:,data:, andvbscript:URL schemes in attributes<svg>,<iframe>,<object>, and<embed>tags with embedded scripts- Event handlers beyond those matched by the simple regex
The comment acknowledges this limitation but doesn't address it. For production use, replace with a robust HTML sanitization library like DOMPurify.
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
585-589: Validate structuredContent against output schema instead of bypassing validation with unsafe cast.The code assigns
uiResult.structuredContent(which containsrawOutput) with an unsafeas Record<string, unknown>cast. SincerawOutputis typed asany, the cast doesn't enforce that the value is actually an object at runtime—ifrawOutputis a primitive or array, the cast will succeed but MCP protocol validation may fail.Instead of relying on the type assertion, validate
uiResult.structuredContentagainst the tool's output schema, or ensure thatrawOutputis validated as an object before being set asstructuredContent.Based on learnings: "Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of
unknownforexecute()andread()methods"libs/ui/src/components/modal.ts (2)
13-29: Add @example tag to JSDoc.The
isSafeUrlfunction is missing@exampledocumentation to illustrate usage, as previously requested and required by coding guidelines.🔎 Add JSDoc example
/** * Validates URL scheme to prevent XSS via javascript: URLs. * * Allowed protocols: * - `http://`, `https://` - Web URLs * - `/`, `#` - Relative paths and anchors * - `mailto:` - Email links * - `tel:` - Phone links * * Blocked protocols (XSS vectors): * - `javascript:` - Inline script execution * - `data:` - Data URIs (can contain scripts) * - `vbscript:` - Legacy script protocol * + * @example + * isSafeUrl('https://example.com') // true + * isSafeUrl('javascript:alert(1)') // false (logs warning) + * isSafeUrl('/relative/path') // true + * isSafeUrl('tel:+1234567890') // true + * * @param url - URL to validate * @returns true if URL uses a safe protocol, false otherwise */As per coding guidelines.
236-238: Document security constraints forconfirmHref.The
confirmHrefproperty lacks security documentation. Developers need to understand which protocols are allowed and what happens when an unsafe URL is provided.🔎 Add security documentation
/** Icon */ icon?: string; - /** Confirm action URL */ + /** + * Confirm action URL + * + * @remarks Only safe protocols are allowed: `http://`, `https://`, `/`, `#`, `mailto:`, `tel:` + * Other protocols (e.g., `javascript:`, `data:`) will be rejected and fall back to button behavior. + */ confirmHref?: string; }Based on past review comments.
🧹 Nitpick comments (1)
libs/ui/src/components/modal.ts (1)
102-186: Add JSDoc@exampletags formodalandconfirmModalfunctions.Both functions lack usage examples in their JSDoc, which is required by coding guidelines to demonstrate basic usage and options patterns.
Example structure for modal function
/** * Build a modal component * * @example * ```typescript * modal('<p>Modal content</p>', { * id: 'my-modal', * title: 'Example Modal', * size: 'md' * }) * ``` */ export function modal(content: string, options: ModalOptions): string { // ... }As per coding guidelines.
Also applies to: 243-325
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/sdk/src/tool/flows/call-tool.flow.ts(7 hunks)libs/ui/src/components/modal.ts(5 hunks)libs/ui/src/universal/renderers/html.renderer.ts(1 hunks)libs/ui/src/universal/renderers/markdown.renderer.ts(1 hunks)libs/ui/src/universal/renderers/mdx.renderer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/ui/src/components/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/components/**/*.ts: Validate component options usingvalidateOptions()helper and return error box on validation failure
Add JSDoc examples with @example tags showing basic usage and options patterns for all components
Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Files:
libs/ui/src/components/modal.ts
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/components/modal.tslibs/ui/src/universal/renderers/html.renderer.tslibs/ui/src/universal/renderers/mdx.renderer.tslibs/ui/src/universal/renderers/markdown.renderer.ts
libs/ui/src/components/**
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Organize components into schema.ts (definitions) and implementation .ts files with matching names
Files:
libs/ui/src/components/modal.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/ui/src/components/modal.tslibs/ui/src/universal/renderers/html.renderer.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/universal/renderers/mdx.renderer.tslibs/ui/src/universal/renderers/markdown.renderer.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/ui/src/components/modal.tslibs/ui/src/universal/renderers/html.renderer.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/universal/renderers/mdx.renderer.tslibs/ui/src/universal/renderers/markdown.renderer.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/tool/flows/call-tool.flow.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
libs/ui/src/components/modal.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
libs/ui/src/components/modal.tslibs/ui/src/universal/renderers/html.renderer.tslibs/ui/src/universal/renderers/mdx.renderer.tslibs/ui/src/universal/renderers/markdown.renderer.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/ui/src/components/modal.tslibs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
libs/ui/src/components/modal.tslibs/ui/src/universal/renderers/html.renderer.tslibs/ui/src/universal/renderers/mdx.renderer.tslibs/ui/src/universal/renderers/markdown.renderer.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
libs/ui/src/components/modal.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/universal/renderers/html.renderer.tslibs/ui/src/universal/renderers/mdx.renderer.tslibs/ui/src/universal/renderers/markdown.renderer.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
🧬 Code graph analysis (1)
libs/sdk/src/tool/flows/call-tool.flow.ts (3)
libs/ui/src/registry/tool-ui.registry.ts (1)
isUIRenderFailure(91-93)libs/ui/src/registry/index.ts (1)
isUIRenderFailure(45-45)libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)
🪛 Biome (2.1.2)
libs/ui/src/universal/renderers/html.renderer.ts
[error] 46-46: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 94-94: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
libs/ui/src/universal/renderers/mdx.renderer.ts
[error] 121-121: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
libs/ui/src/universal/renderers/markdown.renderer.ts
[error] 163-163: Avoid passing children using a prop
The canonical way to pass children in React is to use additional arguments to React.createElement
(lint/correctness/noChildrenProp)
[error] 178-178: Avoid passing children using a prop
The canonical way to pass children in React is to use additional arguments to React.createElement
(lint/correctness/noChildrenProp)
[error] 188-188: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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 (14)
libs/ui/src/universal/renderers/html.renderer.ts (1)
75-97: Good approach, but depends on fixingsanitizeHtml().The
safeHtmlRenderercorrectly attempts to sanitize content before rendering, which is the right pattern. However, its security depends on fixing thesanitizeHtml()function (see previous comment). Once that function uses a robust library like DOMPurify, this renderer will provide proper XSS protection.libs/ui/src/universal/renderers/markdown.renderer.ts (3)
29-40: LGTM: Good URL validation implementation.The
isSafeUrl()function properly validates URL schemes to prevent XSS attacks viajavascript:,data:, andvbscript:URLs. The case-insensitive check is appropriate.Minor suggestion: Consider explicitly checking for null/undefined at the start:
if (!url) return false;This makes the guard more explicit (though the current code works correctly).
131-155: LGTM: Robust markdown detection logic.The
canHandle()method correctly detects markdown patterns while excluding JSX tags to avoid conflicts with the MDX renderer. The auto-detection logic is well-designed.
199-218: LGTM: Clean factory pattern for extensibility.The
createMarkdownRenderer()function provides a good pattern for injecting default components while allowing runtime override through context. The component merging order is correct (defaults → context → content).libs/ui/src/universal/renderers/mdx.renderer.ts (6)
16-24: LGTM: Proper runtime detection.The
getMDXRuntime()function correctly checks for the MDX runtime availability. The use ofanyfor window globals is acceptable in this context.
29-51: LGTM: Comprehensive MDX syntax detection.The
containsMdxSyntax()function correctly identifies MDX-specific patterns (JSX components, expressions, imports, and JSX attributes) to distinguish MDX from plain markdown.
72-87: LGTM: Well-structured content detection.The
canHandle()method correctly identifies MDX content through explicit type checking and auto-detection usingcontainsMdxSyntax(). The logic properly requires both markdown and MDX syntax for auto-detection.
89-163: LGTM: Excellent fallback handling and proper XSS prevention.The render method correctly addresses previous review concerns:
- ✅ Uses
escapeHtml()utility (line 104) as required by coding guidelines- ✅ Uses positional children arguments (line 93)
- ✅ Provides graceful fallbacks for missing MDX runtime (lines 99-124) and uncompiled content (lines 153-161)
The
dangerouslySetInnerHTMLat line 121 is safe because the content is properly escaped viaescapeHtml()at line 104 before rendering. The Biome warning is a false positive in this case.Good defensive programming with the multiple fallback paths and clear console warnings.
170-172: LGTM: Simple and effective utility.The
isMdxSupported()function provides a clean public API for checking MDX runtime availability.
180-198: LGTM: Consistent factory pattern.The
createMdxRenderer()function follows the same extensibility pattern ascreateMarkdownRenderer(), providing consistency across the renderer implementations. Component merging order is correct.libs/sdk/src/tool/flows/call-tool.flow.ts (2)
464-484: LGTM! Well-structured graceful degradation for UI rendering failures.The type guard correctly identifies render failures and proceeds without UI content, maintaining tool functionality while logging diagnostic information. The platform-specific HTML key extraction with fallback to 'ui/html' ensures compatibility across different client types.
556-567: Error propagation is working correctly as designed.The finalize stage returns silently when
toolorrawOutputis undefined because the flow framework uses asuppressErrorsflag during finalize to prevent masking errors from the execute stage. When execute fails, finalize runs in cleanup mode with error suppression enabled, then the original error is re-thrown. This is the correct and safe pattern already implemented in the flow execution logic.libs/ui/src/components/modal.ts (2)
30-46: LGTM! Security validation correctly implemented.The
isSafeUrlfunction properly validates URL schemes and blocks dangerous protocols (javascript:,data:,vbscript:), effectively preventing XSS attacks via href attributes.
291-306: LGTM! Secure URL handling with proper fallback.The confirm button logic correctly validates
confirmHrefusingisSafeUrlbefore rendering an anchor tag, and safely falls back to a button when the URL is missing or unsafe. The use ofescapeHtmlon all dynamic content provides defense in depth.
…ng in markdown renderer
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
585-589: Unsafe cast persists - runtime type mismatch possible.The
as Record<string, unknown>cast doesn't guaranteeuiResult.structuredContentis actually an object at runtime. If the tool'srawOutputis a primitive or array, this cast will pass TypeScript checks but may violate MCP protocol expectations.Consider adding a runtime check:
🔎 Proposed validation
if (uiResult.structuredContent !== undefined && uiResult.structuredContent !== null) { + // Validate structuredContent is an object (MCP protocol requirement) + if (typeof uiResult.structuredContent === 'object' && !Array.isArray(uiResult.structuredContent)) { result.structuredContent = uiResult.structuredContent as Record<string, unknown>; + } else { + this.logger.warn('finalize: structuredContent is not an object, skipping', { + tool: tool.metadata.name, + type: typeof uiResult.structuredContent, + }); + } }
🧹 Nitpick comments (1)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
606-650: Path traversal protection is adequate.The implementation correctly validates that custom paths are within
tempDirorcwdusing proper path boundary checks (+ path.sep). This addresses the previous security concern.Consider top-level imports and directory creation.
Minor improvements for robustness:
- The
require()calls could be top-level ESM imports to avoid runtime cost on each invocation- If
DEBUG_TOOL_RESPONSE_PATHcontains non-existent subdirectories,writeFilewill fail silently🔎 Optional: Add recursive directory creation
+ // Ensure parent directory exists + const dir = path.dirname(outputPath); + await fs.mkdir(dir, { recursive: true }).catch(() => {}); + // Use async write to avoid blocking the event loop (fire-and-forget) fs.writeFile(outputPath, JSON.stringify(debugOutput, null, 2))
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/sdk/src/tool/flows/call-tool.flow.ts(7 hunks)libs/ui/src/universal/renderers/markdown.renderer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/tool/flows/call-tool.flow.tslibs/ui/src/universal/renderers/markdown.renderer.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/tool/flows/call-tool.flow.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/tool/flows/call-tool.flow.tslibs/ui/src/universal/renderers/markdown.renderer.ts
libs/ui/src/**/*.ts
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/**/*.ts: Always useescapeHtml()utility for all user-provided content to prevent XSS vulnerabilities
Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code
Files:
libs/ui/src/universal/renderers/markdown.renderer.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
libs/sdk/src/tool/flows/call-tool.flow.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/ui/src/universal/renderers/markdown.renderer.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
libs/ui/src/universal/renderers/markdown.renderer.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
libs/ui/src/universal/renderers/markdown.renderer.ts
🧬 Code graph analysis (2)
libs/sdk/src/tool/flows/call-tool.flow.ts (3)
libs/ui/src/registry/tool-ui.registry.ts (1)
isUIRenderFailure(91-93)libs/ui/src/registry/index.ts (1)
isUIRenderFailure(45-45)libs/ui/src/adapters/response-builder.ts (1)
buildToolResponseContent(130-212)
libs/ui/src/universal/renderers/markdown.renderer.ts (2)
libs/ui/src/utils/escape-html.ts (1)
escapeHtml(27-41)libs/ui/src/universal/types.ts (3)
ClientRenderer(132-148)UniversalContent(26-40)RenderContext(113-126)
🪛 Biome (2.1.2)
libs/ui/src/universal/renderers/markdown.renderer.ts
[error] 182-182: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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 (9)
libs/sdk/src/tool/flows/call-tool.flow.ts (6)
27-27: LGTM!The import of
isUIRenderFailuretype guard is correctly added and properly used for graceful degradation handling in theapplyUIstage.
416-416: LGTM!The rename from
useDualPayloadtouseStructuredContentaligns with thebuildToolResponseContentAPI signature.
488-507: LGTM!The
useStructuredContentparameter is correctly passed tobuildToolResponseContentand logged for debugging.
545-550: LGTM!The updated JSDoc clearly documents the behavior when
executefails, explaining why finalization is skipped rather than throwing a new error.
556-567: LGTM!The early returns for missing tool/output prevent cascading errors during cleanup. The original execution error will propagate correctly through the flow framework.
464-484: Graceful degradation pattern is solid.The UI render failure handling correctly allows tool execution to proceed without UI when rendering fails. The type guard usage is appropriate.
Platform-specific HTML key mapping is correct as designed.
The htmlKey logic correctly defaults to
frontmcp/htmlfor Claude, Gemini, and other non-OpenAI/non-ext-apps platforms. This is intentional per the codebase strategy (documented in tools-list.flow.ts). Consider extracting the platform-key mapping to a constant or helper function for better readability and to consolidate the logic currently duplicated across flows.libs/ui/src/universal/renderers/markdown.renderer.ts (3)
61-108: Excellent security implementation! Previous review feedback properly addressed.The use of
escapeHtml()at line 63 correctly addresses the previous review comment about manual HTML escaping. The security implementation is solid:
- User content is escaped before markdown pattern replacements
- Links are validated via
isSafeUrl()to block XSS vectors (javascript:, data:, vbscript:)- The fallback parser provides safe rendering when ReactMarkdown is unavailable
126-185: Perfect! React.createElement usage corrected per previous review.The render method now correctly passes children as positional arguments to
React.createElement:
- Line 160: Error message passed as third argument ✓
- Line 174: Markdown source passed as third argument ✓
The
dangerouslySetInnerHTMLusage (lines 180-183) is safe here because the content has been sanitized throughparseMarkdownToHtml(), which appliesescapeHtml()and validates URLs withisSafeUrl(). This is the appropriate fallback when ReactMarkdown is unavailable.
193-212: LGTM! Clean factory pattern with proper component merging.The factory function correctly:
- Merges default components with context components
- Maintains proper precedence order (defaultComponents overridden by context.components)
- Delegates to the base renderer implementation
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/demo/tsconfig.app.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/tsconfig.app.json
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
apps/demo/tsconfig.app.json
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Follow the preset pattern for hierarchical configurations across the codebase
Applied to files:
apps/demo/tsconfig.app.json
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
apps/demo/tsconfig.app.json
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
apps/demo/tsconfig.app.json
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/common/records/**/*.ts : Centralize record types in common/records and import from there instead of from module-specific files
Applied to files:
apps/demo/tsconfig.app.json
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
apps/demo/tsconfig.app.json
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/draft/docs/getting-started/welcome.mdx (1)
39-39: Consider adding a brief explanation of thehttpconfiguration.The example introduces
http: { port: 3000 }without explaining its purpose. Since this is the welcome page and likely the first code example new users will encounter, adding a sentence about what this HTTP server configuration does would improve clarity. For instance, you could add a comment above line 39 or extend the "Core concepts" section to mention HTTP transport.💡 Suggested addition
You could add a brief inline comment:
@FrontMcp({ info: { name: 'Demo 🚀', version: '0.1.0' }, apps: [CalcApp], + // Expose the MCP server over HTTP on port 3000 http: { port: 3000 }, })Or extend the "Core concepts" section to include:
- HTTP Transport — expose your server over HTTP with configurable ports
Based on coding guidelines, this is the correct location (draft/docs/**) for documentation edits.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/draft/docs.json(6 hunks)docs/draft/docs/authentication/demo-servers.mdx(4 hunks)docs/draft/docs/getting-started/quickstart.mdx(2 hunks)docs/draft/docs/getting-started/welcome.mdx(1 hunks)docs/draft/docs/guides/ast-guard.mdx(1 hunks)docs/draft/docs/plugins/cache-plugin.mdx(2 hunks)docs/draft/docs/servers/discovery.mdx(1 hunks)docs/draft/docs/servers/prompts.mdx(1 hunks)docs/draft/docs/servers/resources.mdx(1 hunks)docs/draft/docs/servers/tools.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/draft/docs/plugins/cache-plugin.mdx
- docs/draft/docs/servers/prompts.mdx
- docs/draft/docs/servers/resources.mdx
🧰 Additional context used
📓 Path-based instructions (2)
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/servers/discovery.mdxdocs/draft/docs/servers/tools.mdxdocs/draft/docs/getting-started/quickstart.mdxdocs/draft/docs/guides/ast-guard.mdxdocs/draft/docs/authentication/demo-servers.mdxdocs/draft/docs/getting-started/welcome.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/servers/discovery.mdxdocs/draft/docs/servers/tools.mdxdocs/draft/docs/getting-started/quickstart.mdxdocs/draft/docs/guides/ast-guard.mdxdocs/draft/docs/authentication/demo-servers.mdxdocs/draft/docs/getting-started/welcome.mdxdocs/draft/docs.json
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
docs/draft/docs/servers/discovery.mdxdocs/draft/docs/getting-started/welcome.mdx
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
docs/draft/docs/servers/discovery.mdx
🔇 Additional comments (7)
docs/draft/docs/servers/discovery.mdx (1)
11-11: LGTM! Documentation cross-reference improvement.The explicit, capitalized link text enhances readability and consistency across the discovery documentation. The note clearly directs readers to the appropriate pages for authoring guidance while maintaining focus on MCP wire format details.
docs/draft/docs/authentication/demo-servers.mdx (3)
56-62: LGTM! Transport configuration correctly reflects the API restructuring.The
transportconfiguration is now properly shown as a top-level field, aligning with the API changes that moved it out of the nestedauth.transportstructure. The settings appropriately demonstrate public mode with stateful and streamable HTTP enabled.
90-97: LGTM! Orchestrated mode correctly demonstrates sessionMode configuration.The transport configuration properly includes
sessionMode: 'stateful'(line 92), which aligns with the API restructuring that moved sessionMode to the top-level transport object. This is appropriate for the stateful OAuth flow used in orchestrated mode.
124-130: LGTM! Transport configuration is consistent with the API restructuring.The top-level
transportfield correctly reflects the moved configuration structure, maintaining consistency across all three demo examples.docs/draft/docs.json (2)
36-36: LGTM! Comprehensive SEO keywords added.The expanded keyword list effectively covers the project's core features and technologies, which should improve discoverability for users searching for MCP-related frameworks, authentication solutions, and UI tooling.
90-91: All newly referenced documentation pages exist in the repository. The navigation structure additions are properly backed by documentation files and will not result in broken links.docs/draft/docs/getting-started/quickstart.mdx (1)
345-345: Navigation links have been verified and exist.Both target pages are correctly set up in the documentation:
/docs/authentication/overviewexists with proper content about the three-tier authentication system/docs/extensibility/pluginsexists with proper documentation about plugin extensionsThe API changes in the code example (lines 468-472) correctly reflect accessing logger and auth info through the context object, consistent with the SDK's FrontMcpContext pattern documented throughout the codebase.
…ty and consistency
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/draft/docs/authentication/demo-servers.mdx (1)
182-182: Fix inconsistent anchor reference.Line 182 still uses the broken
#transport-controlsanchor, while lines 103 and 172 were correctly updated to use#transport. This inconsistency will result in a broken link.🔎 Proposed fix
- <Card title="Transport controls" icon="server" href="/docs/servers/server#transport-controls"> + <Card title="Transport controls" icon="server" href="/docs/servers/server#transport">docs/draft/docs/servers/tools.mdx (1)
41-53: Remove incorrect ToolContext import and extends clauses from all tool class examples.Class-based tools in @frontmcp/sdk should use the @tool decorator without extending a base class. The import of
ToolContexton line 41 is incorrect—it does not exist in the official SDK exports. All tool classes (lines 49, 172, 311, 392, 449, 504, 530, 575, 617, 712, etc.) should remove theextends ToolContextclause and use plain class syntax instead.Update the example to:
import { Tool } from '@frontmcp/sdk'; import { z } from 'zod'; @Tool({ name: 'greet', description: 'Greets a user by name', inputSchema: { name: z.string() }, }) class GreetTool { async execute({ name }: { name: string }) { return `Hello, ${name}!`; } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/draft/docs/authentication/demo-servers.mdx(4 hunks)docs/draft/docs/getting-started/quickstart.mdx(2 hunks)docs/draft/docs/servers/tools.mdx(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/draft/docs/getting-started/quickstart.mdx
🧰 Additional context used
📓 Path-based instructions (2)
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/servers/tools.mdxdocs/draft/docs/authentication/demo-servers.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/servers/tools.mdxdocs/draft/docs/authentication/demo-servers.mdx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Support platform-aware rendering by detecting network capabilities (open vs blocked) and script loading strategies (external vs inline)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (5)
docs/draft/docs/authentication/demo-servers.mdx (4)
56-62: LGTM! Transport configuration correctly moved to top-level.The transport configuration has been appropriately extracted from the
authobject to a top-level option, aligning with the API changes described in this PR.
90-97: LGTM! Transport and sessionMode correctly relocated.The transport configuration and
sessionModehave been appropriately moved to the top-level transport block, consistent with the public API changes in this PR.
103-103: LGTM! Link reference correctly updated.The link text and anchor have been updated to reflect the transport configuration's new location at the top level, and the anchor
#transportmatches the target heading.
172-172: LGTM! Previous broken anchor issue resolved.The link now correctly uses the
#transportanchor, addressing the concern raised in the previous review about the broken#transport-controlsreference.docs/draft/docs/servers/tools.mdx (1)
172-794: Systematic update to extend ToolContext is consistent.All class-based tool examples throughout the document have been consistently updated to extend
ToolContext. This includes:
CalculateTotalTool(line 172)CreateUserTool(line 311)ContextExampleTool(line 392)TracedTool(line 449)CalculateTool(line 504)CreateGitHubIssueTool(line 530)UpdateUserStatusTool(line 575)WriteFileTool(line 617)GetWeatherTool(line 712)GetOrderTool(line 794)The consistency suggests this is an intentional documentation update. Verify this matches the actual SDK pattern as noted in the previous comment.
… project configuration
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.