feat: Support Tool UI #93
Conversation
WalkthroughAdds a Tool UI subsystem: typed UI metadata, multi-framework template rendering (HTML/React/MDX), renderer registry and caches, ui:// resource handling with ToolUIRegistry caching, platform detection in sessions, scope/transport integrations, demo Weather apps, extensive docs and tests, and testing helpers for UI assertions and matchers. Changes
sequenceDiagram
participant Client
participant Transport as Transport Layer
participant Scope
participant ToolExec as Tool Execution Flow
participant UIReg as ToolUIRegistry
participant ResourceFlow as ReadResourceFlow
Client->>Transport: callTool(toolName, input, headers)
Transport->>Scope: createSessionId(protocol, token, { userAgent, platformDetectionConfig })
Scope->>Scope: detectPlatformFromUserAgent / notifications -> platformType
Transport->>ToolExec: invoke tool execution (includes session payload)
ToolExec->>ToolExec: execute tool.execute(input) -> output
alt tool.metadata.ui present
ToolExec->>UIReg: renderAndRegister({ toolName, input, output, uiConfig, platformType, requestId })
UIReg->>UIReg: render template (HTML/React/MDX) -> html
UIReg->>UIReg: buildUIMeta(html, uiConfig, platformType) -> meta, resourceUri
UIReg-->>ToolExec: { uri, html, meta }
ToolExec->>Transport: return tool response with _meta (ui resource info)
Client->>ResourceFlow: readResource(ui://tools/{tool}/{requestId})
ResourceFlow->>UIReg: handleUIResourceRead(uri, registry, platformType)
UIReg-->>ResourceFlow: ReadResourceResult (cached html / placeholder / error)
ResourceFlow-->>Client: return resource contents (mimeType + html)
else
ToolExec->>Transport: return normal tool response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)docs/**⚙️ CodeRabbit configuration file
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
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: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
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)
117-119: MutatingrawInputviolates flow state guidelines.Line 118 mutates
this.rawInputdirectly to store_toolOwnerId. Based on learnings, flows should usestate.set()for managing flow state instead of mutatingrawInput.Consider storing this in the flow state instead:
// Store tool owner ID in the flow input for hook filtering if (tool?.owner) { - (this.rawInput as any)._toolOwnerId = tool.owner.id; + // Note: This should ideally be stored in state, but would require + // updating hook filtering to read from state. Keeping for compatibility. }Alternatively, add
toolOwnerIdto the state schema (similar to howresourceOwnerIdis handled inread-resource.flow.tsat line 39).
🟡 Minor comments (15)
libs/sdk/src/resource/flows/resource-templates-list.flow.ts-173-194 (1)
173-194: Avoid non-null assertions and extract duplicated _meta augmentation logic.The non-null assertions at lines 175 and 187 violate coding guidelines. While the filter guarantees
uriTemplateis non-null, use type narrowing instead:- const resourceTemplates: ResponseTemplateItem[] = resolved - .filter(({ template }) => template.uriTemplate != null) - .map(({ finalName, template }) => { - const item: ResponseTemplateItem = { - uriTemplate: template.uriTemplate!, // Guaranteed by filter above + const resourceTemplates: ResponseTemplateItem[] = resolved + .filter(({ template }) => template.uriTemplate != null) + .map(({ finalName, template }) => { + const uriTemplate = template.uriTemplate as string; + const item: ResponseTemplateItem = { + uriTemplate, name: finalName, title: template.metadata.title, description: template.metadata.description, mimeType: template.metadata.mimeType, icons: template.metadata.icons, }; if (template.metadata.mimeType === 'text/html+skybridge') { item._meta = { - 'openai/outputTemplate': template.uriTemplate!, + 'openai/outputTemplate': uriTemplate, 'openai/resultCanProduceWidget': true, 'openai/widgetAccessible': true, }; } return item; });Additionally, this _meta augmentation logic is duplicated in
resources-list.flow.ts. Extract to a shared utility to reduce duplication:function addOpenAIMetadata<T extends { mimeType?: string; _meta?: Record<string, unknown> }>( item: T, uri: string, mimeType?: string ): T { if (mimeType === 'text/html+skybridge') { item._meta = { 'openai/outputTemplate': uri, 'openai/resultCanProduceWidget': true, 'openai/widgetAccessible': true, }; } return item; }libs/sdk/src/resource/flows/resources-list.flow.ts-175-196 (1)
175-196: Avoid non-null assertions - use proper type narrowing instead.The non-null assertions on lines 177 and 189 (
resource.uri!) violate the coding guideline to avoid non-null assertions. While the preceding filter at line 174 checks thatresource.uri != null, TypeScript's type narrowing doesn't carry across the separate.map()callback function.Use a type guard or extract the guaranteed-non-null value before the map:
-.filter(({ resource }) => resource.uri != null) -.map(({ finalName, resource }) => { - const item: ResponseResourceItem = { - uri: resource.uri!, +.filter(({ resource }) => resource.uri !== null && resource.uri !== undefined) +.map(({ finalName, resource }) => { + // uri is now properly narrowed + const item: ResponseResourceItem = { + uri: resource.uri,Or better, extract the value with a type guard before mapping:
-.filter(({ resource }) => resource.uri != null) -.map(({ finalName, resource }) => { +.map(({ finalName, resource }) => { + if (!resource.uri) return null; const item: ResponseResourceItem = { uri: resource.uri,Additionally, the OpenAI-specific metadata on lines 185-193 is tightly coupled to the core resource flow. Consider extracting this into a separate, configurable metadata augmentation layer to avoid platform-specific logic in generic SDK code.
Committable suggestion skipped: line range outside the PR's diff.
libs/sdk/src/scope/flows/http.request.flow.ts-203-213 (1)
203-213: Missing return afterthis.next()causes unintended fall-through.When
decision.intent === 'unknown'for an authorized request,this.next()is called but execution continues to set the intent on line 213. This will throw sincethis.next()(aFlowControl) throws, but the logic suggests areturnwas intended.if (decision.intent === 'unknown') { // continue to other middleware // with authentication (public/authorized routes) this.logger.verbose(`[${this.requestId}] decision is unknown, continue to next http middleware`); this.next(); + return; }apps/demo/src/apps/weather/tools/get-weather.tool.ts-125-140 (1)
125-140: Avoid non-null assertions on Partial properties.Per coding guidelines, avoid non-null assertions (
!). TheweatherDatais typed asPartial<WeatherOutput>, so these properties could be undefined. Use nullish coalescing with defaults instead.Apply this diff:
- let temperature = weatherData.temperature!; + let temperature = weatherData.temperature ?? 20; // Convert to Fahrenheit if requested if (units === 'fahrenheit') { temperature = Math.round((temperature * 9) / 5 + 32); } return { location: input.location, temperature, units, - conditions: weatherData.conditions!, - humidity: weatherData.humidity!, - windSpeed: weatherData.windSpeed!, - icon: weatherData.icon!, + conditions: weatherData.conditions ?? 'unknown', + humidity: weatherData.humidity ?? 50, + windSpeed: weatherData.windSpeed ?? 10, + icon: weatherData.icon ?? 'cloudy', };libs/sdk/src/common/decorators/tool.decorator.ts-167-167 (1)
167-167: Remove unused imports:ToolUIConfigandTemplateContext.These imports on line 167 are not used in the file.
TemplateHelpersis used in theui.templatecontext type definition (line 195), butToolUIConfigandTemplateContextare defined inline instead.-import type { ToolUIConfig, TemplateContext, TemplateHelpers } from '../metadata/tool-ui.metadata'; +import type { TemplateHelpers } from '../metadata/tool-ui.metadata';libs/ui/src/base-template/default-base-template.ts-200-207 (1)
200-207: Potential null reference if polyfill not loaded.
checkForData()callswindow.__frontmcp.getToolOutput()without verifying thatwindow.__frontmcpexists. If polyfills fail to load or are disabled, this throws.function checkForData() { + if (!window.__frontmcp || typeof window.__frontmcp.getToolOutput !== 'function') { + return false; + } var data = window.__frontmcp.getToolOutput(); if (data !== undefined) { render(data); return true; } return false; }libs/ui/src/base-template/polyfills.ts-109-127 (1)
109-127: Missing HTTP response status check before parsing JSON.If the server returns a non-2xx response,
response.json()may still succeed but produce unexpected results, or fail with a confusing parse error. Checkresponse.okfirst.var response = await fetch(session.mcpUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-Session-Id': session.sessionId }, body: JSON.stringify({ jsonrpc: '2.0', method: 'tools/call', params: { name: toolName, arguments: args || {} }, id: Date.now() }) }); + if (!response.ok) { + throw new Error('HTTP ' + response.status + ': ' + response.statusText); + } + var result = await response.json();libs/ui/src/base-template/polyfills.ts-88-88 (1)
88-88: Unused variableoriginalCallTool.The original
callToolis captured but never used. If this was intended for chaining to a previous implementation, the fallback logic is missing. Otherwise, remove the dead code.- var originalCallTool = window.__frontmcp.callTool; window.__frontmcp.callTool = async function(toolName, args) {libs/sdk/src/notification/notification.service.ts-100-103 (1)
100-103:'google'keyword may cause false positives for non-Gemini Google clients.Clients like "google-drive-connector" or "google-sheets-mcp" would be detected as 'gemini'. Consider being more specific or ordering the check after more specific patterns.
// Google Gemini clients - if (lowerIdentifier.includes('gemini') || lowerIdentifier.includes('google') || lowerIdentifier.includes('bard')) { + if (lowerIdentifier.includes('gemini') || lowerIdentifier.includes('bard')) { return 'gemini'; }If
libs/sdk/src/notification/notification.service.ts-64-76 (1)
64-76: String matching comment is misleading - usesincludes()not exact match.The comment says "Exact string match (case-insensitive)" but the code uses
includes()which is substring matching. This may cause unintended matches (e.g., pattern "gpt" would match "chatgpt-unofficial-client").Consider updating the comment or the logic:
if (typeof mapping.pattern === 'string') { - // Exact string match (case-insensitive) + // Substring match (case-insensitive) if (lowerClientName.includes(mapping.pattern.toLowerCase())) { return mapping.platform; }Or if exact match is intended:
if (typeof mapping.pattern === 'string') { // Exact string match (case-insensitive) - if (lowerClientName.includes(mapping.pattern.toLowerCase())) { + if (lowerClientName === mapping.pattern.toLowerCase()) { return mapping.platform; }libs/ui/src/runtime/csp.ts-120-134 (1)
120-134: Wildcard domain regex rejects valid single-character domain segments.The regex
[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]requires at least 2 characters for the domain part, rejecting valid domains likehttps://*.x.ioorhttps://*.a.co.- if (domain.startsWith('https://*.')) { - const rest = domain.slice(10); - return /^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]\.[a-zA-Z]{2,}$/.test(rest); - } + if (domain.startsWith('https://*.')) { + const rest = domain.slice(10); + // Allow single-char segments and handle multi-level domains + return /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?\.[a-zA-Z]{2,}$/.test(rest); + }libs/ui/src/runtime/csp.ts-104-114 (1)
104-114: ReplaceescapeAttribute()with importedescapeHtml()utility.The local
escapeAttribute()function has identical implementation to the sharedescapeHtml()utility exported from../layouts/base. Per coding guidelines, use the canonicalescapeHtml()utility for all HTML-related escaping to maintain consistency across the codebase and avoid duplication.Import
escapeHtmlfrom../layouts/baseand replace the function call at line 73.libs/ui/src/runtime/sanitizer.ts-183-189 (1)
183-189: Silent fallback when depth limit exceeded could leak PII.When the recursion depth exceeds
maxDepth, the function returns the value unchanged without any warning. In deeply nested structures, this could silently leak PII that would otherwise be redacted.Consider logging a warning or throwing an error when depth is exceeded with PII still present:
// Prevent infinite recursion if (depth > maxDepth) { + // Consider warning in development or throwing for critical use cases return value; }Committable suggestion skipped: line range outside the PR's diff.
libs/sdk/src/tool/ui/ui-resource.handler.ts-328-340 (1)
328-340: Factory doesn't forwardplatformTypeto handler.
createUIResourceHandlercreates a handler that callshandleUIResourceRead(uri, registry)without the optionalplatformTypeparameter. This means the MIME type will always default totext/html+skybridgeregardless of the actual platform.Consider accepting and forwarding
platformType:export interface UIResourceHandlerOptions { /** ToolUIRegistry instance */ registry: ToolUIRegistry; + /** Platform type for MIME selection */ + platformType?: AIPlatformType; /** Optional custom error handler */ onError?: (error: string, uri: string) => void; } export function createUIResourceHandler(options: UIResourceHandlerOptions) { - const { registry, onError } = options; + const { registry, platformType, onError } = options; return function handleUIResource(uri: string): UIResourceHandleResult { - const result = handleUIResourceRead(uri, registry); + const result = handleUIResourceRead(uri, registry, platformType);Committable suggestion skipped: line range outside the PR's diff.
libs/ui/src/runtime/sanitizer.ts-273-288 (1)
273-288: Type inconsistency in defensive guard.The function signature claims to return
Record<string, unknown>, but the defensive guard at lines 277-279 can returnnull,undefined, or primitive values unchanged. This could cause runtime type confusion for callers.Consider either:
- Throwing an error for invalid input:
if (!input || typeof input !== 'object') { - return input; + throw new Error('sanitizeInput requires a non-null object'); }
- Or adjusting the return type to reflect reality:
-): Record<string, unknown> { +): Record<string, unknown> | null | undefined {
🧹 Nitpick comments (22)
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (2)
40-44: Structured error logging is good; just be mindful of verbosity/sensitivity in productionThe structured
logger.error('CallTool Failed', { tool, error: { name, message, stack } })is a nice improvement over opaque error logging and should greatly help debugging without changing the returned MCP error.Depending on your deployment environment, you may want to confirm that including full stacks here is acceptable from a verbosity and information-disclosure standpoint (e.g., log level tuning or stack redaction in production).
22-37: UseCallToolResultSchemafor robust runtime validation ofFlowControl.respondoutputThe current validation checks only that
outputis an object with acontentproperty before casting toCallToolResult. While better than an unchecked cast, this manual shape-checking is loose and may miss structural issues. The MCP SDK exportsCallToolResultSchemaas a Zod schema specifically for this purpose.Replace the manual validation with schema-based validation:
-import { CallToolRequest, CallToolRequestSchema, CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import { CallToolRequest, CallToolRequestSchema, CallToolResult, CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js'; if (e.type === 'respond') { - // Validate output before casting - if (e.output && typeof e.output === 'object' && 'content' in e.output) { - return e.output as CallToolResult; + const parsed = CallToolResultSchema.safeParse(e.output); + if (parsed.success) { + return parsed.data; } logger.error('FlowControl.respond has invalid output', { tool: toolName, - outputType: typeof e.output, - hasOutput: !!e.output, - outputKeys: e.output && typeof e.output === 'object' ? Object.keys(e.output) : [], + issues: parsed.error.issues, }); return formatMcpErrorResponse(new InternalMcpError('FlowControl output is not a valid CallToolResult')); }Additionally, line 36's
logger.warnfires for all non-respondoutcomes. Ifhandledandnextare expected control flow patterns, downgrade those toinfoordebugand reservewarnfor genuinely exceptional types likefailorabort.libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts (1)
54-54: Avoid non-null assertion onserverOptions.capabilities.Per coding guidelines, avoid non-null assertions (
!) and use proper error handling instead. IfserverOptions.capabilitiescould be undefined, either validate it earlier or provide a default.return { - capabilities: serverOptions.capabilities!, + capabilities: serverOptions.capabilities ?? {}, instructions: serverOptions.instructions,Alternatively, if capabilities are required, throw a descriptive error earlier in the handler if they're missing.
libs/sdk/src/scope/flows/http.request.flow.ts (1)
216-221: Consider adding explicit return afterthis.next()for clarity.While
this.next()throwsFlowControlpreventing fall-through, an explicitreturnimproves readability and makes the control flow intention clear.if (decision.intent === 'unknown') { this.logger.verbose(`[${this.requestId}] decision is unknown, continue to other public http middleware`); // continue to other middleware // without authentication (public routes) this.next(); + return; }libs/ui/src/runtime/types.ts (1)
123-124: Consider removing unnecessary type alias.
UICSPTypeis only used once (line 274). Consider usingUIContentSecurityPolicydirectly to reduce indirection.-// Local alias for use within this file -type UICSPType = UIContentSecurityPolicy;And update line 274:
- csp?: UICSPType; + csp?: UIContentSecurityPolicy;apps/demo/src/main.ts (1)
5-5: Remove unused import.The
CrmMcpAppimport is no longer used after replacing it withWeatherMcpAppon Line 10.Apply this diff:
-import CrmMcpApp from './apps/crm';libs/sdk/src/common/interfaces/tool.interface.ts (2)
120-133: Add defensive null check for authInfo.The getter accesses
this.authInfo.sessionIdPayloadandthis.authInfo.sessionIdwithout verifying thatauthInfoitself is defined. WhileauthInfois likely always present in normal execution, adding a defensive check would align with the null-safe pattern used elsewhere.Apply this diff to add a defensive check:
get platform(): AIPlatformType { + if (!this.authInfo) { + return 'unknown'; + } + // First check sessionIdPayload (detected from user-agent during session creation) const payloadPlatform = this.authInfo.sessionIdPayload?.platformType; if (payloadPlatform && payloadPlatform !== 'unknown') { return payloadPlatform; } // Fall back to notification service (detected from MCP clientInfo during initialize) const sessionId = this.authInfo.sessionId; if (!sessionId) { return 'unknown'; } return this.scope.notifications.getPlatformType(sessionId); }
149-155: Add defensive null check for authInfo.The getter accesses
this.authInfo.sessionIdwithout verifying thatauthInfoitself is defined. Apply the same defensive pattern used in the platform getter.Apply this diff:
get clientInfo(): ClientInfo | undefined { + if (!this.authInfo) { + return undefined; + } + const sessionId = this.authInfo.sessionId; if (!sessionId) { return undefined; } return this.scope.notifications.getClientInfo(sessionId); }libs/sdk/src/common/metadata/tool.metadata.ts (1)
13-14: Consider strengthening runtime validation for the ui field.The
uifield has a strongly-typed TypeScript interface (ToolUIConfig<...>) on Line 219, but the Zod schema on Line 262 usesz.looseObject({})which accepts any properties. This creates a gap between compile-time type safety and runtime validation.If
ToolUIConfighas a corresponding Zod schema (e.g.,toolUIConfigSchema), consider referencing it for stricter runtime validation:- ui: z.looseObject({}).optional(), + ui: toolUIConfigSchema.optional(),If the flexible validation is intentional to support dynamic UI configurations, consider adding a comment explaining this design choice.
Also applies to: 219-219, 262-262
libs/sdk/src/notification/__tests__/platform-detection.test.ts (1)
1-135: Good test coverage for coredetectAIPlatformbehavior, but some gaps remain.The tests thoroughly cover the default platform detection across all platform families with good use of parameterized testing.
Consider adding tests for:
detectPlatformFromUserAgent— This function is exported fromnotification.servicebut not tested here.- Custom mappings config — The
detectAIPlatformfunction accepts aPlatformDetectionConfigparameter withmappingsandcustomOnlyoptions that aren't exercised.- Edge cases — e.g., client names containing multiple platform keywords like
"openai-claude-client".Per learnings, testing behavior across platform configurations is important.
Example additions:
describe('custom mappings', () => { it('should use custom mapping when provided', () => { const config = { mappings: [{ pattern: /my-custom-client/i, platform: 'openai' as const }] }; expect(detectAIPlatform({ name: 'my-custom-client', version: '1.0' }, config)).toBe('openai'); }); it('should return unknown when customOnly is true and no match', () => { const config = { customOnly: true, mappings: [] }; expect(detectAIPlatform({ name: 'ChatGPT', version: '1.0' }, config)).toBe('unknown'); }); });apps/demo/src/apps/weather/tools/get-weather.tool.ts (1)
14-18: Consider using.strict()on the input schema object.Based on patterns in the codebase (e.g.,
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts), input schemas typically use.strict()to reject unknown properties.const inputSchema = { location: z.string().describe('City name or location'), units: z.enum(['celsius', 'fahrenheit']).optional().describe('Temperature units'), -}; +} as const;Note: If the SDK's
@Tooldecorator requires a plain object shape rather than a ZodObject, this is acceptable. The demo file primarily serves as a usage example.libs/sdk/src/tool/ui/template-helpers.ts (1)
56-61: Hardcoded'en-US'locale limits i18n support.The
formatCurrencyfunction hardcodes the locale. Consider accepting an optionallocaleparameter for broader i18n compatibility.-export function formatCurrency(amount: number, currency = 'USD'): string { - return new Intl.NumberFormat('en-US', { +export function formatCurrency(amount: number, currency = 'USD', locale = 'en-US'): string { + return new Intl.NumberFormat(locale, { style: 'currency', currency, }).format(amount); }libs/ui/src/runtime/mcp-bridge.ts (1)
19-19: Unused type imports.The imported types
ProviderType,ThemeMode,DisplayMode, andHostContextare not used in this file sinceMCP_BRIDGE_RUNTIMEis a string constant. These can be removed unless intended for future use.-import type { ProviderType, ThemeMode, DisplayMode, HostContext } from './types';libs/ui/src/runtime/csp.ts (1)
146-148: Consider sanitizing domain before logging.The domain value is logged directly to console. While
console.warnis generally safe, logging user-provided values without sanitization is a minor hygiene concern for structured logging systems.} else { - console.warn(`Invalid CSP domain ignored: ${domain}`); + console.warn(`Invalid CSP domain ignored: ${domain.slice(0, 100)}`); }libs/sdk/src/tool/ui/tool-ui.registry.ts (2)
300-318: LRU eviction uses insertion order rather than access order.The eviction logic at line 310 uses
[...this.cache.keys()].slice(0, toRemove)which removes the oldest by insertion order. However, the comment says "LRU-style eviction". True LRU would track last access time and evict least-recently-accessed entries.Current behavior evicts entries that were created first, which is FIFO, not LRU. This is acceptable for many use cases, but the comment is misleading.
Consider updating the comment to reflect actual behavior:
- // Remove ~10% of entries (oldest first by insertion order) + // Remove ~10% of entries (FIFO - oldest created first) const toRemove = Math.ceil(MAX_CACHE_SIZE * 0.1); const keys = [...this.cache.keys()].slice(0, toRemove);
224-241: O(n) cache scan forgetLatestForToolmay become a bottleneck.This method iterates over all cache entries to find the latest for a tool. With
MAX_CACHE_SIZE = 1000, this is acceptable. If cache size increases significantly, consider maintaining a secondary index (Map<toolName, uri[]>) for O(1) lookups.libs/ui/src/runtime/sanitizer.ts (1)
60-64: Phone pattern may produce false positives.The phone regex is quite permissive and could match sequences that aren't phone numbers (e.g., "123-456-7890" in a product code). This is acceptable for auto-detection mode where false positives are tolerable, but consider documenting this limitation for users who need precise detection.
libs/sdk/src/tool/ui/ui-resource.handler.ts (1)
270-295: Redundant cache lookup logic.The code first calls
registry.getCachedEntry(uri)at line 271, and if it returnsundefined, it callsregistry.getRenderedHtml(uri)at line 274. However, looking atToolUIRegistry(in the relevant snippets), both methods perform the same cache lookup and expiry check. ThegetRenderedHtmlcall will always returnundefinedifgetCachedEntryalready returnedundefined.The comment at line 273 suggests this handles "different URI format" cases, but both methods use the exact same URI key.
Consider simplifying:
// Try to get cached HTML from the registry by exact URI const cachedEntry = registry.getCachedEntry(uri); if (!cachedEntry) { - // Also try to get just by the HTML (in case cached with different URI format) - const html = registry.getRenderedHtml(uri); - if (!html) { - return { - handled: true, - error: `UI resource not found or expired: ${uri}`, - }; - } - - // Return the HTML as a resource return { handled: true, - result: { - contents: [ - { - uri, - mimeType, - text: html, - }, - ], - }, + error: `UI resource not found or expired: ${uri}`, }; }libs/ui/src/runtime/wrapper.ts (2)
311-317:wrapToolUIMinimaldoesn't supportsanitizeInputoption.Unlike
wrapToolUI, the minimal wrapper doesn't accept or apply thesanitizeInputoption. This inconsistency could lead to accidental PII exposure when users switch between the two wrappers.Consider adding sanitization support for consistency:
export function wrapToolUIMinimal( options: Pick< - WrapToolUIOptions, + WrapToolUIFullOptions, - 'content' | 'toolName' | 'input' | 'output' | 'structuredContent' | 'csp' | 'widgetAccessible' | 'title' + 'content' | 'toolName' | 'input' | 'output' | 'structuredContent' | 'csp' | 'widgetAccessible' | 'title' | 'sanitizeInput' >, ): string { - const { content, toolName, input = {}, output, structuredContent, csp, widgetAccessible = false, title } = options; + const { content, toolName, input = {}, output, structuredContent, csp, widgetAccessible = false, title, sanitizeInput: sanitizeOption } = options; + // Apply input sanitization if enabled + let sanitizedInput = input; + if (sanitizeOption) { + const sanitizeMode = sanitizeOption === true ? true : sanitizeOption; + sanitizedInput = sanitizeInputFn(input, sanitizeMode); + }
405-414: Inconsistent platform type withgetUIResourceMimeType.This function uses a 3-value union (
'openai' | 'ext-apps' | 'generic'), whilegetUIResourceMimeTypeinui-resource.handler.tsuses the fullAIPlatformType(8 values includingclaude,cursor, etc.). This could cause confusion about which function to use and may result in incorrect MIME types for platforms like Claude.Consider aligning with
AIPlatformTypeor clearly documenting when to use each function:+import type { AIPlatformType } from '@frontmcp/sdk'; // or appropriate path -export function getToolUIMimeType(platform: 'openai' | 'ext-apps' | 'generic' = 'generic'): string { +export function getToolUIMimeType(platform: AIPlatformType | 'ext-apps' = 'generic-mcp'): string { switch (platform) { case 'openai': return 'text/html+skybridge'; case 'ext-apps': return 'text/html+mcp'; + case 'claude': + case 'gemini': + return 'text/html'; default: - return 'text/html'; + return 'text/html+skybridge'; } }libs/ui/src/tool-template/builder.ts (2)
144-144: Remove unnecessary type assertion.The type assertion
ui as ToolUIConfigis unnecessary becauseToolUIConfig<In, Out>is structurally compatible withToolUIConfig(without generics), since the generic parameters only affect the template function signature and TypeScript uses structural typing.Apply this diff:
- const openaiMeta = buildOpenAIMetaFromUI(ui as ToolUIConfig); + const openaiMeta = buildOpenAIMetaFromUI(ui);
247-293: Excellent XSS prevention with one minor note.All user-provided text content is properly escaped using
escapeHtml(), which follows the coding guidelines forlibs/ui/src/**/*.ts. TheclassNameparameters incontainer,paragraph, andkeyValueare interpolated directly, but this is acceptable since class names are typically developer-controlled constants, not user input.If there's any scenario where user-provided content could be passed as
className, consider adding validation or documentation to clarify that these parameters must be trusted, developer-provided values only.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
apps/demo/src/apps/weather/index.ts(1 hunks)apps/demo/src/apps/weather/tools/get-weather.tool.ts(1 hunks)apps/demo/src/main.ts(1 hunks)libs/sdk/src/auth/session/utils/session-id.utils.ts(2 hunks)libs/sdk/src/common/decorators/tool.decorator.ts(1 hunks)libs/sdk/src/common/interfaces/tool.interface.ts(2 hunks)libs/sdk/src/common/metadata/index.ts(1 hunks)libs/sdk/src/common/metadata/tool-ui.metadata.ts(1 hunks)libs/sdk/src/common/metadata/tool.metadata.ts(3 hunks)libs/sdk/src/common/tokens/tool.tokens.ts(1 hunks)libs/sdk/src/common/types/auth/session.types.ts(3 hunks)libs/sdk/src/common/types/options/session.options.ts(3 hunks)libs/sdk/src/completion/flows/complete.flow.ts(2 hunks)libs/sdk/src/notification/__tests__/platform-detection.test.ts(1 hunks)libs/sdk/src/notification/index.ts(1 hunks)libs/sdk/src/notification/notification.service.ts(4 hunks)libs/sdk/src/resource/flows/read-resource.flow.ts(8 hunks)libs/sdk/src/resource/flows/resource-templates-list.flow.ts(1 hunks)libs/sdk/src/resource/flows/resources-list.flow.ts(1 hunks)libs/sdk/src/resource/resource.registry.ts(1 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(9 hunks)libs/sdk/src/scope/scope.instance.ts(4 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(3 hunks)libs/sdk/src/tool/flows/tools-list.flow.ts(2 hunks)libs/sdk/src/tool/ui/index.ts(1 hunks)libs/sdk/src/tool/ui/platform-adapters.ts(1 hunks)libs/sdk/src/tool/ui/render-template.ts(1 hunks)libs/sdk/src/tool/ui/template-helpers.ts(1 hunks)libs/sdk/src/tool/ui/tool-ui.registry.ts(1 hunks)libs/sdk/src/tool/ui/ui-resource-template.ts(1 hunks)libs/sdk/src/tool/ui/ui-resource.handler.ts(1 hunks)libs/sdk/src/transport/flows/handle.sse.flow.ts(1 hunks)libs/sdk/src/transport/flows/handle.streamable-http.flow.ts(4 hunks)libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts(2 hunks)libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts(1 hunks)libs/ui/src/base-template/default-base-template.ts(1 hunks)libs/ui/src/base-template/index.ts(1 hunks)libs/ui/src/base-template/polyfills.ts(1 hunks)libs/ui/src/base-template/theme-styles.ts(1 hunks)libs/ui/src/index.ts(1 hunks)libs/ui/src/runtime/csp.ts(1 hunks)libs/ui/src/runtime/index.ts(1 hunks)libs/ui/src/runtime/mcp-bridge.ts(1 hunks)libs/ui/src/runtime/sanitizer.ts(1 hunks)libs/ui/src/runtime/types.ts(1 hunks)libs/ui/src/runtime/wrapper.ts(1 hunks)libs/ui/src/tool-template/builder.ts(1 hunks)libs/ui/src/tool-template/index.ts(1 hunks)
🧰 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:
libs/sdk/src/common/metadata/index.tslibs/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/common/types/options/session.options.tslibs/sdk/src/tool/ui/template-helpers.tslibs/ui/src/index.tslibs/sdk/src/common/tokens/tool.tokens.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/ui/src/tool-template/index.tslibs/ui/src/base-template/default-base-template.tslibs/ui/src/runtime/mcp-bridge.tslibs/ui/src/base-template/index.tsapps/demo/src/apps/weather/index.tslibs/sdk/src/tool/ui/render-template.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/ui/src/base-template/theme-styles.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tsapps/demo/src/main.tslibs/sdk/src/resource/resource.registry.tslibs/ui/src/runtime/csp.tslibs/sdk/src/tool/ui/tool-ui.registry.tslibs/sdk/src/completion/flows/complete.flow.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/notification/index.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/ui/platform-adapters.tslibs/sdk/src/common/types/auth/session.types.tslibs/sdk/src/common/metadata/tool-ui.metadata.tsapps/demo/src/apps/weather/tools/get-weather.tool.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/tool/ui/index.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/ui/src/runtime/types.tslibs/sdk/src/tool/ui/ui-resource.handler.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/ui/src/runtime/wrapper.tslibs/sdk/src/tool/ui/ui-resource-template.tslibs/ui/src/tool-template/builder.tslibs/ui/src/runtime/sanitizer.tslibs/sdk/src/auth/session/utils/session-id.utils.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/ui/src/runtime/index.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/common/metadata/index.tslibs/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/common/types/options/session.options.tslibs/sdk/src/tool/ui/template-helpers.tslibs/sdk/src/common/tokens/tool.tokens.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/sdk/src/tool/ui/render-template.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/ui/tool-ui.registry.tslibs/sdk/src/completion/flows/complete.flow.tslibs/sdk/src/notification/index.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/ui/platform-adapters.tslibs/sdk/src/common/types/auth/session.types.tslibs/sdk/src/common/metadata/tool-ui.metadata.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/tool/ui/index.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/ui/ui-resource.handler.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/sdk/src/tool/ui/ui-resource-template.tslibs/sdk/src/auth/session/utils/session-id.utils.tslibs/sdk/src/transport/flows/handle.streamable-http.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/common/metadata/index.tslibs/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/common/types/options/session.options.tslibs/sdk/src/tool/ui/template-helpers.tslibs/ui/src/index.tslibs/sdk/src/common/tokens/tool.tokens.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/ui/src/tool-template/index.tslibs/ui/src/base-template/default-base-template.tslibs/ui/src/runtime/mcp-bridge.tslibs/ui/src/base-template/index.tslibs/sdk/src/tool/ui/render-template.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/ui/src/base-template/theme-styles.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/resource/resource.registry.tslibs/ui/src/runtime/csp.tslibs/sdk/src/tool/ui/tool-ui.registry.tslibs/sdk/src/completion/flows/complete.flow.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/notification/index.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/ui/platform-adapters.tslibs/sdk/src/common/types/auth/session.types.tslibs/sdk/src/common/metadata/tool-ui.metadata.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/tool/ui/index.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/ui/src/runtime/types.tslibs/sdk/src/tool/ui/ui-resource.handler.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/ui/src/runtime/wrapper.tslibs/sdk/src/tool/ui/ui-resource-template.tslibs/ui/src/tool-template/builder.tslibs/ui/src/runtime/sanitizer.tslibs/sdk/src/auth/session/utils/session-id.utils.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/ui/src/runtime/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/index.tslibs/ui/src/tool-template/index.tslibs/ui/src/base-template/default-base-template.tslibs/ui/src/runtime/mcp-bridge.tslibs/ui/src/base-template/index.tslibs/ui/src/base-template/theme-styles.tslibs/ui/src/runtime/csp.tslibs/ui/src/base-template/polyfills.tslibs/ui/src/runtime/types.tslibs/ui/src/runtime/wrapper.tslibs/ui/src/tool-template/builder.tslibs/ui/src/runtime/sanitizer.tslibs/ui/src/runtime/index.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
**/*.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/sdk/src/notification/__tests__/platform-detection.test.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/apps/weather/index.tsapps/demo/src/main.tsapps/demo/src/apps/weather/tools/get-weather.tool.ts
🧠 Learnings (19)
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/common/metadata/index.tslibs/ui/src/index.tslibs/ui/src/tool-template/index.tslibs/ui/src/base-template/index.tslibs/sdk/src/tool/ui/index.tslibs/ui/src/runtime/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 behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/common/types/options/session.options.tslibs/ui/src/index.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/ui/src/runtime/mcp-bridge.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/tool/ui/platform-adapters.tslibs/ui/src/runtime/types.tslibs/sdk/src/auth/session/utils/session-id.utils.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/common/types/options/session.options.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/tool/ui/platform-adapters.tslibs/sdk/src/notification/notification.service.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/transport/flows/handle.sse.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/types/options/session.options.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/notification/notification.service.tslibs/ui/src/runtime/types.tslibs/ui/src/runtime/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 : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
libs/sdk/src/tool/ui/template-helpers.tslibs/ui/src/runtime/csp.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/sdk/src/tool/ui/template-helpers.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.tslibs/ui/src/base-template/index.tslibs/sdk/src/common/metadata/tool-ui.metadata.tslibs/sdk/src/tool/ui/index.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/tool-template/index.tslibs/ui/src/base-template/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/theme/presets/**/*.ts : Implement GitHub/OpenAI gray-black monochromatic palette as the default theme
Applied to files:
libs/ui/src/base-template/default-base-template.tslibs/ui/src/base-template/index.tslibs/ui/src/base-template/theme-styles.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/ui/src/runtime/mcp-bridge.tslibs/ui/src/base-template/polyfills.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/runtime/mcp-bridge.tslibs/ui/src/base-template/polyfills.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/base-template/index.tslibs/ui/src/base-template/theme-styles.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/base-template/theme-styles.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/completion/flows/complete.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/common/metadata/tool.metadata.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/tools-list.flow.tslibs/sdk/src/common/metadata/tool.metadata.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/sdk/src/resource/flows/read-resource.flow.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/sdk/src/scope/scope.instance.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 : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
libs/sdk/src/common/metadata/tool.metadata.ts
🧬 Code graph analysis (20)
libs/sdk/src/common/interfaces/tool.interface.ts (4)
libs/sdk/src/common/types/auth/session.types.ts (1)
AIPlatformType(13-21)libs/sdk/src/notification/index.ts (2)
AIPlatformType(12-12)ClientInfo(11-11)libs/sdk/src/notification/notification.service.ts (2)
AIPlatformType(19-19)ClientInfo(44-49)libs/testing/src/client/mcp-test-client.ts (1)
sessionId(206-208)
libs/sdk/src/common/types/options/session.options.ts (1)
libs/sdk/src/common/types/auth/session.types.ts (2)
AIPlatformType(13-21)aiPlatformTypeSchema(26-35)
libs/sdk/src/tool/ui/template-helpers.ts (3)
libs/ui/src/runtime/wrapper.ts (1)
createTemplateHelpers(79-137)libs/ui/src/runtime/types.ts (1)
TemplateHelpers(40-55)libs/sdk/src/common/metadata/tool-ui.metadata.ts (1)
TemplateHelpers(40-70)
libs/sdk/src/notification/__tests__/platform-detection.test.ts (3)
libs/sdk/src/notification/index.ts (3)
detectAIPlatform(13-13)AIPlatformType(12-12)ClientInfo(11-11)libs/sdk/src/notification/notification.service.ts (3)
detectAIPlatform(164-182)AIPlatformType(19-19)ClientInfo(44-49)libs/sdk/src/common/interfaces/tool.interface.ts (1)
clientInfo(149-155)
libs/ui/src/base-template/default-base-template.ts (3)
libs/ui/src/base-template/index.ts (7)
BaseTemplateOptions(16-16)McpSession(28-28)createDefaultBaseTemplate(14-14)ThemeStylesOptions(24-24)renderThemeStyles(21-21)renderMcpSessionPolyfill(28-28)createMinimalBaseTemplate(15-15)libs/ui/src/base-template/polyfills.ts (2)
McpSession(13-18)renderMcpSessionPolyfill(32-150)libs/ui/src/base-template/theme-styles.ts (2)
ThemeStylesOptions(23-36)renderThemeStyles(66-101)
libs/ui/src/runtime/mcp-bridge.ts (1)
libs/ui/src/runtime/index.ts (3)
MCP_BRIDGE_RUNTIME(32-32)getMCPBridgeScript(32-32)isMCPBridgeSupported(32-32)
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (1)
libs/testing/src/interceptor/interceptor-chain.ts (1)
logger(185-190)
libs/sdk/src/scope/flows/http.request.flow.ts (3)
libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowControl(17-41)libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
decideIntent(397-451)libs/sdk/src/common/schemas/http-output.schema.ts (1)
httpRespond(256-338)
libs/sdk/src/tool/flows/call-tool.flow.ts (2)
libs/sdk/src/tool/ui/render-template.ts (1)
hasUIConfig(60-63)libs/sdk/src/scope/scope.instance.ts (1)
Scope(34-225)
libs/sdk/src/resource/resource.registry.ts (2)
libs/sdk/src/resource/resource.utils.ts (3)
isResourceTemplate(143-160)normalizeResourceTemplate(115-138)normalizeResource(87-110)libs/sdk/src/resource/resource.instance.ts (1)
ResourceInstance(26-192)
libs/sdk/src/completion/flows/complete.flow.ts (1)
libs/sdk/src/tool/ui/render-template.ts (1)
hasUIConfig(60-63)
libs/sdk/src/tool/flows/tools-list.flow.ts (2)
libs/sdk/src/tool/ui/index.ts (1)
hasUIConfig(13-13)libs/sdk/src/tool/ui/render-template.ts (1)
hasUIConfig(60-63)
libs/sdk/src/common/types/auth/session.types.ts (2)
libs/sdk/src/notification/index.ts (1)
AIPlatformType(12-12)libs/sdk/src/notification/notification.service.ts (1)
AIPlatformType(19-19)
libs/sdk/src/common/metadata/tool-ui.metadata.ts (1)
libs/ui/src/runtime/types.ts (6)
UIContentSecurityPolicy(19-31)TemplateHelpers(40-55)TemplateContext(60-72)TemplateBuilderFn(77-77)WidgetServingMode(82-86)ToolUIConfig(91-121)
apps/demo/src/apps/weather/tools/get-weather.tool.ts (2)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)libs/sdk/src/common/interfaces/tool.interface.ts (4)
output(79-81)output(83-86)input(62-64)input(66-73)
libs/sdk/src/notification/notification.service.ts (4)
libs/sdk/src/notification/index.ts (4)
ClientInfo(11-11)AIPlatformType(12-12)detectPlatformFromUserAgent(14-14)detectAIPlatform(13-13)libs/sdk/src/common/types/options/session.options.ts (2)
PlatformMappingEntry(11-16)PlatformDetectionConfig(21-33)libs/sdk/src/common/types/auth/session.types.ts (1)
AIPlatformType(13-21)libs/sdk/src/common/interfaces/tool.interface.ts (1)
clientInfo(149-155)
libs/sdk/src/tool/ui/ui-resource.handler.ts (2)
libs/sdk/src/tool/ui/tool-ui.registry.ts (1)
ToolUIRegistry(106-319)libs/sdk/src/common/types/auth/session.types.ts (1)
AIPlatformType(13-21)
libs/sdk/src/tool/ui/ui-resource-template.ts (1)
libs/sdk/src/tool/ui/index.ts (2)
StaticWidgetResourceTemplate(53-53)ToolUIResourceTemplate(53-53)
libs/ui/src/tool-template/builder.ts (5)
libs/sdk/src/tool/ui/render-template.ts (2)
RenderTemplateOptions(13-22)renderToolTemplate(31-54)libs/ui/src/runtime/index.ts (4)
ToolUIConfig(27-27)TemplateContext(24-24)createTemplateHelpers(49-49)TemplateBuilderFn(26-26)libs/ui/src/runtime/types.ts (3)
ToolUIConfig(91-121)TemplateContext(60-72)TemplateBuilderFn(77-77)libs/sdk/src/tool/ui/template-helpers.ts (2)
createTemplateHelpers(86-94)escapeHtml(16-26)libs/ui/src/runtime/wrapper.ts (1)
createTemplateHelpers(79-137)
libs/sdk/src/auth/session/utils/session-id.utils.ts (4)
libs/sdk/src/common/types/options/session.options.ts (1)
PlatformDetectionConfig(21-33)libs/sdk/src/common/types/auth/session.types.ts (3)
TransportProtocolType(7-7)AIPlatformType(13-21)SessionIdPayload(69-84)libs/sdk/src/notification/index.ts (2)
AIPlatformType(12-12)detectPlatformFromUserAgent(14-14)libs/sdk/src/notification/notification.service.ts (2)
AIPlatformType(19-19)detectPlatformFromUserAgent(136-154)
…atform detection comments
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
libs/ui/src/runtime/sanitizer.ts (1)
319-360: Add circular reference protection to prevent infinite loops.While the depth limit addresses the previous review comment, the function still lacks protection against circular references. An object with
obj.self = objwill recurse unnecessarily until hitting the depth limit.Apply this diff to add circular reference protection:
export function detectPII( input: Record<string, unknown>, options?: { maxDepth?: number }, ): { hasPII: boolean; fields: string[]; } { const maxDepth = options?.maxDepth ?? 10; const fields: string[] = []; + const visited = new WeakSet<object>(); const check = (value: unknown, path: string[], depth: number): void => { // Prevent stack overflow on deeply nested structures if (depth > maxDepth) return; if (value === null || value === undefined) return; if (typeof value === 'string') { if (detectPIIType(value)) { fields.push(path.join('.')); } return; } if (Array.isArray(value)) { + if (visited.has(value)) return; + visited.add(value); value.forEach((item, index) => check(item, [...path, String(index)], depth + 1)); return; } if (typeof value === 'object') { + if (visited.has(value)) return; + visited.add(value); for (const [k, v] of Object.entries(value as Record<string, unknown>)) { check(v, [...path, k], depth + 1); } } }; check(input, [], 0); return { hasPII: fields.length > 0, fields, }; }libs/ui/src/runtime/mcp-bridge.ts (1)
327-347: postMessage origin validation still allows spoofed responses beforetrustedOriginis setYou’ve added
trustedOriginchecks, but for ext‑apps there’s still a window where any origin can respond to_sendRequestbeforetrustedOriginis established, including the initialui/initializeresponse that setshostContextandtrustedOrigin. A malicious ancestor frame could spoof a JSON‑RPC response during that phase and control both context and the origin you subsequently trust.To harden this:
- Establish
trustedOriginbased on configuration or first JSON‑RPC message before processing it, and- Avoid trusting
result.trustedOriginfrom the unvalidated payload.For example, inside the embedded script:
- // Trusted origin for postMessage validation (set during ext-apps initialization) - var trustedOrigin = null; + // Trusted origin for postMessage validation (set via host context or on first message) + var trustedOrigin = (window.__mcpHostContext && window.__mcpHostContext.origin) || null; ... - // Store trusted origin for message validation - if (result && result.trustedOrigin) { - trustedOrigin = result.trustedOrigin; - } ... - window.addEventListener('message', function(event) { - // Validate origin for ext-apps environment to prevent spoofed messages - if (isExtApps && trustedOrigin && event.origin !== trustedOrigin) { - console.warn('MCP Bridge: Ignoring message from untrusted origin:', event.origin); - return; - } - - var data = event.data; - if (!data || data.jsonrpc !== '2.0') return; + window.addEventListener('message', function(event) { + var data = event.data; + if (!data || data.jsonrpc !== '2.0') return; + + // Validate / establish trusted origin for ext-apps environment + if (isExtApps) { + if (!trustedOrigin) { + // Trust-on-first-use; alternatively, require configured hostContext.origin in prod + trustedOrigin = event.origin; + } else if (event.origin !== trustedOrigin) { + console.warn('MCP Bridge: Ignoring message from untrusted origin:', event.origin); + return; + } + }Optionally, you can also switch the
postMessagetarget from'*'totrustedOrigin || '*'in_sendRequestand_initExtAppsoncetrustedOriginis known. This keeps the pendingRequests logic intact while tightening the origin story. Based on past review comments and coding guidelines for secure libs/ui runtimes.Also applies to: 372-377
🧹 Nitpick comments (11)
libs/sdk/src/common/decorators/tool.decorator.ts (1)
167-168: Move import to the top of the file with other imports.Placing imports mid-file breaks conventional module organization and can confuse readers and tooling. Move this import to the top, after line 14.
import { ToolContext } from '../interfaces'; +import type { TemplateHelpers } from '../metadata/tool-ui.metadata';Then remove lines 167-168.
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
409-409: Remove redundantString()wrapper.The
requestIdis already typed asstring(line 396), so theString()wrapper is unnecessary.Apply this diff:
- requestId: String(requestId), + requestId,libs/sdk/src/scope/flows/http.request.flow.ts (1)
88-89: Remove redundant type annotations.TypeScript can infer these types from the literal values.
Apply this diff:
- private requestStartTime: number = 0; - private requestId: string = ''; + private requestStartTime = 0; + private requestId = '';libs/ui/src/runtime/sanitizer.ts (1)
61-61: Remove unnecessary escape characters in regex.The escape characters
\+and\.are unnecessary inside the character class. In a character class,+and.are literal characters and don't need escaping.Apply this diff:
- phone: /^[\+]?[(]?[0-9]{1,4}[)]?[-\s\.]?[0-9]{1,4}[-\s\.]?[0-9]{1,9}$/, + phone: /^[+]?[(]?[0-9]{1,4}[)]?[-\s.]?[0-9]{1,4}[-\s.]?[0-9]{1,9}$/,libs/sdk/src/notification/__tests__/platform-detection.test.ts (1)
1-137: Good coverage of default client-name detection; consider adding config & user-agent testsThe table-driven tests for the main vendors, generic MCP, unknown clients, and case-insensitive matching are solid and read well. To fully exercise the new detection surface, it would be worth adding a few focused cases:
detectAIPlatformwithPlatformDetectionConfig.mappings:
- One string pattern mapping (e.g., exact
"ChatGPT"→'openai').- One
RegExpmapping.- A
customOnly: truescenario (both matching and non-matching) to ensure fallback is correctly skipped.detectPlatformFromUserAgent:
- A couple of real-ish UA strings (e.g., containing
openai-mcp,claude-desktop,gemini) to verify parity withdefaultPlatformDetection.- A
customOnly: truecase here as well.A sketch of the additional patterns you might add:
+ describe('custom mappings', () => { + it('uses string mappings before default detection', () => { + const clientInfo: ClientInfo = { name: 'MyCustomClient', version: '1.0.0' }; + const config = { + mappings: [{ pattern: 'MyCustomClient', platform: 'openai' }], + }; + expect(detectAIPlatform(clientInfo, config)).toBe('openai'); + }); + + it('honors customOnly by not falling back', () => { + const clientInfo: ClientInfo = { name: 'UnknownClient', version: '1.0.0' }; + const config = { customOnly: true, mappings: [] }; + expect(detectAIPlatform(clientInfo, config)).toBe('unknown'); + }); + }); + + describe('detectPlatformFromUserAgent', () => { + it('detects platform from UA using default rules', () => { + expect(detectPlatformFromUserAgent('openai-mcp/1.0.0')).toBe('openai'); + }); + });This should help keep overall coverage high and ensure the config knobs behave as expected.
libs/sdk/src/notification/notification.service.ts (2)
81-187: Default/platform detection helpers look consistent; consider tests for config paths and UA flowThe factoring here is clean:
defaultPlatformDetectioncentralizes keyword-based detection and is reused from bothdetectPlatformFromUserAgentanddetectAIPlatform.- OpenAI/Claude/Gemini/Cursor/Continue/Cody/generic-MCP ordering makes sense, and the Gemini branch’s
google-ai/google aifilter matches the comment about avoiding genericgoogle-*false positives.- Both
detectPlatformFromUserAgentanddetectAIPlatformcorrectly:
- Short-circuit on missing input to
'unknown'.- Check custom mappings first.
- Respect
customOnlyby skipping fallback detection.Functionally this looks solid. The main follow-up I’d recommend is test coverage that hits:
detectPlatformFromUserAgentdirectly (includingcustomOnlyand a custom mapping).detectAIPlatformwith both string andRegExpmappings once the string semantics are finalized as per the previous comment.These tests will also guard against regressions if the keyword list or ordering changes later.
217-229: ClientInfo/platformType wiring in NotificationService is straightforward; ensure public API is documentedThe
RegisteredServeradditions and the new NotificationService methods:
- Store
clientInfoandplatformTypeon the per-session record.- Derive
platformTypeonce insetClientInfousingscope.metadata?.session?.platformDetectionanddetectAIPlatform, with graceful handling of unregistered sessions (warning +undefined).- Expose
getClientInfoandgetPlatformTypewith sensible defaults ('unknown'when no platform has been detected).This is a good fit with the rest of the service’s session-centric API surface.
Given this is under
libs/sdkand these shapes are now part of the public SDK:
- Make sure
ClientInfo,detectAIPlatform,detectPlatformFromUserAgent, andgetPlatformTypeare captured in the relevantdocs/draft/docs/**pages (usage examples, config knobs, and how platform detection flows through sessions/tools).- Consider adding a brief note in the docs/spec for
scope.metadata.session.platformDetectionso integrators know where to configure mappings.From the code side, the implementation here looks correct and consistent.
Also applies to: 707-755
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (1)
1-6: Typed CallTool handler looks good; consider wiringresponseSchemaas wellUsing
CallToolRequest/CallToolResultplussatisfies McpHandler<CallToolRequest, CallToolResult>is a nice tightening of the handler’s public contract, and the dedicated result schema import fits the MCP schema-first approach.Since you already depend on
CallToolResultSchemaat runtime, it’s worth also exposing it on the handler:return { requestSchema: CallToolRequestSchema, responseSchema: CallToolResultSchema, handler: async (request: CallToolRequest, ctx) => { // ... }, } satisfies McpHandler<CallToolRequest, CallToolResult>;This keeps request/response metadata symmetric and makes it easier for generic tooling to introspect response shapes. Based on learnings, this strengthens the TypeScript-first MCP schema guarantees.
Also applies to: 52-52
libs/ui/src/runtime/mcp-bridge.ts (3)
38-43: Clarify platform detection so Claude/Gemini never get mis‑classified asext-appsRight now
isExtAppsiswindow.parent !== window && !isOpenAI, and the logic inprovider,callTool, andsendFollowUpMessagechecksisExtAppsbeforeisClaude. In an environment where both flags are true (e.g., Claude widget in an iframe), the bridge will treat it as ext‑apps, try to postMessage, and won’t enforce the Claude “network‑blocked” semantics you coded.To make platform behavior deterministic and match the “platform‑aware rendering” goal:
- Prefer specific platforms (Claude, Gemini) over the generic
ext-appsflag.- Ensure
callTool/sendFollowUpMessagereject on Claude even ifisExtAppsis also true.- Ensure
providerreports the concrete platform.For example inside the embedded script:
get provider() { - if (isOpenAI) return 'openai'; - if (isExtApps) return 'ext-apps'; - if (isClaude) return 'claude'; - if (isGemini) return 'gemini'; + if (isOpenAI) return 'openai'; + if (isClaude) return 'claude'; + if (isGemini) return 'gemini'; + if (isExtApps) return 'ext-apps'; return 'unknown'; }, ... callTool: function(name, params) { if (isOpenAI && window.openai && window.openai.callTool) { return window.openai.callTool(name, params); } - if (isExtApps) { - return this._sendRequest('tools/call', { name: name, arguments: params }); - } - if (isClaude) { + if (isClaude) { return Promise.reject(new Error( 'Tool calls are not supported in Claude widgets (network-blocked sandbox)' )); } + if (isExtApps) { + return this._sendRequest('tools/call', { name: name, arguments: params }); + } ... sendFollowUpMessage: function(options) { ... - if (isExtApps) { + if (isClaude) { + return Promise.reject(new Error( + 'Follow-up messages are not supported in Claude widgets (network-blocked sandbox)' + )); + } + if (isExtApps) { return this._sendRequest('ui/message', { role: 'user', content: { type: 'text', text: prompt } }); } - if (isClaude) { - return Promise.reject(new Error( - 'Follow-up messages are not supported in Claude widgets (network-blocked sandbox)' - )); - }You could also refine
isExtAppsitself to explicitly excludeisClaude/isGeminifor extra safety. Based on learnings about platform-aware rendering across OpenAI/Claude/Gemini.Also applies to: 95-101, 191-204, 256-276, 468-472
493-495: AlignisMCPBridgeSupported()detection with the runtime’s platform flags
isMCPBridgeSupported()currently returns true ifwindow.openaiexists, we’re in an iframe, orwindow.claudeexists. The embedded runtime script, however, also checkswindow.__mcpPlatform === 'claude'/'gemini'.To avoid false negatives when the host uses the
__mcpPlatformflag (especially for Gemini), consider mirroring the same conditions here, e.g.:export function isMCPBridgeSupported(): boolean { if (typeof window === 'undefined') return false; const w = window as unknown as { openai?: unknown; claude?: unknown; __mcpPlatform?: string }; return ( typeof w.openai !== 'undefined' || typeof w.claude !== 'undefined' || w.__mcpPlatform === 'claude' || w.__mcpPlatform === 'gemini' || window.parent !== window ); }(or re-use a typed global declaration if you already extend
Windowelsewhere). This keeps the TS helper in sync with the embedded script’s notion of supported platforms. Based on learnings about supporting OpenAI/Claude/Gemini consistently.
387-391: Preserve JSON‑RPC error details when rejecting_sendRequestpromisesThe current error handler discards
error.codeanderror.data, making it difficult for callers to differentiate between error types (e.g., invalid params vs. method not found). Since this is browser runtime code that cannot import backend SDK error classes, preserve the metadata by attaching it to the Error object:if (data.error) { - pending.reject(new Error(data.error.message || 'Unknown error')); + const err = new Error(data.error.message || 'Unknown error'); + err.code = data.error.code; + err.data = data.error.data; + pending.reject(err); } else { pending.resolve(data.result); }This allows consumers to access error metadata for better error handling without requiring TypeScript modifications or SDK dependencies.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/demo/src/apps/weather/tools/get-weather.tool.ts(1 hunks)apps/demo/src/main.ts(1 hunks)libs/sdk/src/common/decorators/tool.decorator.ts(1 hunks)libs/sdk/src/notification/__tests__/platform-detection.test.ts(1 hunks)libs/sdk/src/notification/notification.service.ts(4 hunks)libs/sdk/src/resource/flows/resource-templates-list.flow.ts(1 hunks)libs/sdk/src/resource/flows/resources-list.flow.ts(1 hunks)libs/sdk/src/resource/resource.registry.ts(1 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(9 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(4 hunks)libs/sdk/src/tool/flows/tools-list.flow.ts(2 hunks)libs/sdk/src/tool/ui/ui-resource-template.ts(1 hunks)libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts(1 hunks)libs/ui/src/base-template/polyfills.ts(1 hunks)libs/ui/src/runtime/csp.ts(1 hunks)libs/ui/src/runtime/mcp-bridge.ts(1 hunks)libs/ui/src/runtime/sanitizer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/tool/flows/tools-list.flow.ts
- libs/ui/src/runtime/csp.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/ui/ui-resource-template.tslibs/sdk/src/tool/flows/call-tool.flow.tsapps/demo/src/main.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/notification/notification.service.tslibs/ui/src/runtime/mcp-bridge.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/sdk/src/common/decorators/tool.decorator.tsapps/demo/src/apps/weather/tools/get-weather.tool.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/ui/src/runtime/sanitizer.tslibs/sdk/src/resource/resource.registry.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/ui/ui-resource-template.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/resource/resource.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/sdk/src/tool/ui/ui-resource-template.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/notification/notification.service.tslibs/ui/src/runtime/mcp-bridge.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/notification/__tests__/platform-detection.test.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/ui/src/runtime/sanitizer.tslibs/sdk/src/resource/resource.registry.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/main.tsapps/demo/src/apps/weather/tools/get-weather.tool.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/mcp-bridge.tslibs/ui/src/base-template/polyfills.tslibs/ui/src/runtime/sanitizer.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/sdk/src/notification/__tests__/platform-detection.test.ts
🧠 Learnings (8)
📚 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/sdk/src/tool/ui/ui-resource-template.tslibs/ui/src/base-template/polyfills.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/sdk/src/tool/ui/ui-resource-template.tslibs/ui/src/base-template/polyfills.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/notification/notification.service.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.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/runtime/mcp-bridge.tslibs/sdk/src/notification/__tests__/platform-detection.test.ts
📚 Learning: 2025-12-01T00:33:33.636Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.636Z
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/ui/src/runtime/mcp-bridge.tslibs/ui/src/base-template/polyfills.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.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:
libs/ui/src/runtime/mcp-bridge.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/runtime/mcp-bridge.tslibs/ui/src/base-template/polyfills.ts
🧬 Code graph analysis (5)
libs/sdk/src/tool/ui/ui-resource-template.ts (1)
libs/sdk/src/tool/ui/template-helpers.ts (1)
escapeHtml(16-26)
libs/sdk/src/notification/notification.service.ts (4)
libs/sdk/src/notification/index.ts (4)
ClientInfo(11-11)AIPlatformType(12-12)detectPlatformFromUserAgent(14-14)detectAIPlatform(13-13)libs/sdk/src/common/types/options/session.options.ts (2)
PlatformMappingEntry(11-16)PlatformDetectionConfig(21-33)libs/sdk/src/common/types/auth/session.types.ts (1)
AIPlatformType(13-21)libs/sdk/src/common/interfaces/tool.interface.ts (1)
clientInfo(149-155)
libs/sdk/src/scope/flows/http.request.flow.ts (3)
libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowControl(17-41)libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
decideIntent(397-451)libs/sdk/src/common/schemas/http-output.schema.ts (1)
httpRespond(256-338)
libs/sdk/src/notification/__tests__/platform-detection.test.ts (1)
libs/sdk/src/notification/notification.service.ts (3)
detectAIPlatform(169-187)AIPlatformType(19-19)ClientInfo(44-49)
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (2)
libs/sdk/src/transport/mcp-handlers/mcp-handlers.types.ts (2)
McpHandlerOptions(43-46)McpHandler(26-41)libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowControl(17-41)
🪛 ESLint
libs/sdk/src/scope/flows/http.request.flow.ts
[error] 88-88: Type number trivially inferred from a number literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
[error] 89-89: Type string trivially inferred from a string literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
libs/ui/src/runtime/sanitizer.ts
[error] 61-61: Unnecessary escape character: +.
(no-useless-escape)
[error] 61-61: Unnecessary escape character: ..
(no-useless-escape)
[error] 61-61: Unnecessary escape character: ..
(no-useless-escape)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (31)
libs/sdk/src/resource/resource.registry.ts (1)
520-564: Type safety and dependency graph fixes applied correctly.The implementation now properly:
- Uses
ResourceTypeinstead ofanyfor type safety- Builds the dependency graph with validation matching
buildGraph()'s pattern- Creates instances following the same flow as
initialize()libs/sdk/src/resource/flows/resource-templates-list.flow.ts (1)
173-197: LGTM - Widget metadata augmentation is correctly implemented.The conditional
_metainjection fortext/html+skybridgeresources follows the expected pattern for OpenAI widget discovery. The type assertion on line 175 is appropriately justified by the preceding filter, and the comment clarifies this narrowing.libs/sdk/src/resource/flows/resources-list.flow.ts (1)
175-199: LGTM - Consistent _meta augmentation with the templates list flow.The implementation mirrors
resource-templates-list.flow.tsappropriately, ensuring both resources and resource templates expose the same OpenAI widget metadata structure. The type narrowing pattern is correctly applied.libs/sdk/src/tool/ui/ui-resource-template.ts (3)
21-22: XSS vulnerabilities addressed with proper escaping.The previous review flagged XSS risks from unescaped user params. This is now resolved with the
escapeHtmlimport and its consistent application throughout both templates. Based on learnings, this follows the requirement to always useescapeHtml()for user-provided content.
32-56: LGTM - Static widget template correctly implemented.The template metadata is well-defined, and the fallback handler properly escapes user input. The comments clearly document the expected interception behavior by
ReadResourceFlow.
65-103: LGTM - Dynamic result template with proper error handling.The fallback error HTML provides helpful debugging information while properly escaping all user-provided values (
uri,toolName,requestId). The styling makes the error state clearly visible during development.apps/demo/src/main.ts (1)
1-13: LGTM! Clean demo configuration update.The switch from CrmMcpApp to WeatherMcpApp is straightforward and well-documented. The commented imports provide helpful context about other available demo apps.
apps/demo/src/apps/weather/tools/get-weather.tool.ts (3)
10-32: Excellent type safety and schema definitions.The schemas are well-defined with proper zod types and descriptions. The type inference pattern using
z.infer<z.ZodObject<typeof inputSchema>>correctly handles the SDK's expected input schema format (plain object with zod validators). Good adherence to TypeScript best practices.
45-103: Strong security practices and proper UI component usage.The tool decorator is well-configured with comprehensive metadata. The UI template demonstrates excellent security practices:
- Proper use of
helpers.escapeHtml()for user-provided strings (location, conditions)- Safe interpolation of numeric values (temperature, humidity, windSpeed)
- Correct usage of @frontmcp/ui components (card, descriptionList, badge)
The inline template is lengthy (~44 lines) but appropriate for a demo that serves as an SDK usage example.
104-141: Well-implemented execute method with excellent defensive coding.The implementation demonstrates several best practices:
- Proper type constraints on
ToolContext<typeof inputSchema, typeof outputSchema>- Defensive coding with nullish coalescing operators (
??) throughout (lines 125, 136-139)- Correct Celsius to Fahrenheit conversion formula:
(C × 9/5) + 32(line 129)- Appropriate use of
Partial<WeatherOutput>for mock data flexibility- Safe fallback behavior for unknown locations
Perfect for a demo that showcases SDK usage patterns.
libs/ui/src/base-template/polyfills.ts (3)
13-18: LGTM!The
McpSessioninterface is well-typed with appropriate JSDoc documentation. The exported interface enables proper type checking in dependent modules.
32-153: Well-structured polyfill with proper escaping.The function correctly:
- Escapes user-provided
mcpUrlandsessionIdviaescapeJs()before embedding in the template- Implements a sensible detection cascade (explicit → meta tags → URL params → fallback)
- Provides HTTP fallback with appropriate error handling when
mcpBridgeis unavailable- Uses
'use strict'mode for the generated script
155-169: Security fix applied: escapeJs now handles all injection vectors.The function properly addresses the previously flagged issues:
- Backticks (line 163) — prevents template literal breakout
</scriptsequences (line 166) — prevents script tag injection- Unicode line/paragraph separators (lines 167-168) — prevents line terminator injection
The escape order is correct with backslashes processed first to avoid double-escaping.
libs/sdk/src/common/decorators/tool.decorator.ts (2)
169-175: LGTM!The base type definition correctly wraps
ToolMetadatawith constrained generic parameters, allowing both raw shapes andZodObjectfor input schemas.
181-206: The mentioned UI fields are not present in the codebase and appear to be from a stale summary.Searches for
invocationStatus,servingMode,customWidgetUrl, anddirectPathacross all TypeScript, JSON, and Markdown files return no results. These fields do not exist anywhere in the repository. The currentuiobject implementation inToolMetadataOptions(lines 181-206) is the actual state:template,csp,widgetAccessible,displayMode, andwidgetDescription. No tests, documentation, or related code references the four mentioned fields.Likely an incorrect or invalid review comment.
libs/sdk/src/tool/flows/call-tool.flow.ts (4)
2-2: LGTM - Past review concern addressed.The
randomUUIDimport fromcryptois now present and used correctly on line 396. ThehasUIConfigandScopeimports appropriately support the new UI rendering functionality.Also applies to: 24-25
359-359: LGTM.Adding
inputto the state destructuring is appropriate, as it's used in the UI rendering logic (line 410) and was previously set during theparseInputstage.
443-456: LGTM - Enhanced observability.The detailed logging provides valuable insights into the response structure (content, structuredContent, metadata). Responding with
resultinstead ofparseResult.datais correct sinceresultnow contains the merged UI metadata.
425-427: This behavior is intentional and properly handled—no action needed.The code clears
contentwhenstructuredContentis present, which is by design to avoid redundancy. UI widgets display thestructuredContentdirectly, making the JSON text unnecessary. All downstream consumers safely handle empty content arrays: the UI bridge explicitly tracks both fields separately, the test client uses optional chaining and existence checks, and result extractors check for zero-length arrays before processing.libs/sdk/src/scope/flows/http.request.flow.ts (7)
104-128: LGTM: Comprehensive request tracing.The tracing implementation provides good observability with appropriate truncation of sensitive data (userAgent, sessionId).
130-151: LGTM: Proper FlowControl handling.The try/catch correctly distinguishes between FlowControl exceptions (expected control flow) and actual errors, logging only the latter.
165-243: LGTM: Router logic handles all cases correctly.The enhanced router properly handles:
- DELETE session termination per MCP spec
- Terminated session requests (404 response)
- Protocol detection from session payload
- Unknown intents (forwarding to custom middleware)
- Unauthorized requests (401 with WWW-Authenticate)
252-347: LGTM: Consistent error handling across handlers.All handler stages follow the same pattern: wrap in try/catch, distinguish FlowControl from errors, log non-control errors via
logError, and always rethrow. This ensures proper control flow while capturing actual errors.
356-391: LGTM: Robust DELETE session handling.The implementation correctly validates the session ID, handles the not-found case (404), and returns appropriate responses per MCP spec. Error handling follows the established pattern.
393-457: LGTM: Comprehensive error logging and finalization.The
finalizestage provides request completion logging with duration metrics. ThelogErrorhelper captures rich error context including stack traces, request details, and flow state, while appropriately truncating sensitive data. This significantly improves observability.
178-184: This is not a violation of the coding guideline.ServerRequestTokens.authandServerRequestTokens.sessionIdare official inter-flow communication tokens created viatokenFactory.meta(). Downstream flows (handle.sse.flow.ts, handle.streamable-http.flow.ts, handle.stateless-http.flow.ts) depend on these tokens being present on the request object. The guideline "Don't mutate rawInput in flows - use state.set()" applies to flow control state (verifyResult, intent decisions), which are correctly stored viastate.set()elsewhere in this file. ServerRequestTokens mutations are the established pattern for passing data between flows.Likely an incorrect or invalid review comment.
libs/ui/src/runtime/sanitizer.ts (3)
118-162: LGTM! Well-designed detection utilities.The PII detection functions are properly typed, include appropriate validation logic (e.g., digit count check for credit cards), and use type-safe return values.
167-178: LGTM! Correct pattern replacement order.The function correctly applies more specific patterns first (credit cards, SSNs) before broader patterns (emails, phones), preventing false matches.
1-36: No action required. The review comment conflates two separate security concerns.The sanitizer module correctly handles PII redaction at the data layer. HTML escaping is a rendering layer responsibility implemented through
ctx.helpers.escapeHtml()provided by wrapper.ts to all templates. The coding guideline to useescapeHtml()for user-provided content applies to rendering contexts (templates, components), not to data transformation modules like sanitizer.ts. The sanitized output data correctly flows to templates where developers have access to the escapeHtml helper, and this pattern is consistently applied throughout the codebase. Sanitizer.ts has no responsibility for HTML escaping.Likely an incorrect or invalid review comment.
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (2)
14-15: Scoped logger and tool-name logging are a solid observability improvementCreating a child logger per handler and emitting a verbose
tools/call: <toolName>log is a good way to trace tool activity without polluting global logs. No changes needed here.Also applies to: 19-21
44-48: Structured error logging in the catch block looks appropriateCapturing
tool, plus{ name, message, stack }for thrownErrorinstances (and passing through non-Error values as-is) gives good diagnostic value without over-logging. Delegating the final response shape to the existing error formatter keeps concerns nicely separated.
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (1)
libs/sdk/src/common/decorators/tool.decorator.ts (1)
216-217: UseUIContentSecurityPolicytype instead of inline object definition.This inline type
{ connectDomains?: string[]; resourceDomains?: string[] }duplicates theUIContentSecurityPolicyinterface from../metadata/tool-ui.metadata.ts. Using the shared type improves maintainability and follows the DRY principle.+import type { TemplateHelpers, UIContentSecurityPolicy } from '../metadata/tool-ui.metadata'; -import type { TemplateHelpers } from '../metadata/tool-ui.metadata'; ... - csp?: { connectDomains?: string[]; resourceDomains?: string[] }; + csp?: UIContentSecurityPolicy;
🧹 Nitpick comments (33)
docs/draft/docs/ui/components/card.mdx (2)
153-192: Inconsistent code patterns for headerActions.The Complex Example at line 184 uses a direct function call for
headerActions, which differs from the pattern shown in lines 40–43 where multiple components are wrapped in a template literal. While both may work syntactically, this inconsistency could confuse users about the intended usage pattern.For consistency and clarity, consider wrapping the badge call in a template literal:
- headerActions: badge('Pro', { variant: 'primary', pill: true }), + headerActions: `${badge('Pro', { variant: 'primary', pill: true })}`,This matches the pattern used elsewhere in the documentation (e.g., lines 40–43 and 185–191).
194-235: Clarify HTMX type specification in API reference.Line 217 lists the
htmxoption type asobject, which is vague. Consider providing more specific type information or examples to help users understand the expected structure (e.g., mention that it accepts standard HTMX attributes likeget,trigger,swap).apps/ui/react-demo/src/apps/weather/tools/weather-ui.tsx (3)
33-33: Consider inlining the type or adding a JSDoc comment.The empty interface is flagged by ESLint as redundant. You can either:
- Use the type inline:
({ output, helpers }: ToolUIProps<unknown, WeatherOutput>)- Keep it for readability and add a JSDoc comment explaining its purpose as a type alias
Option 1: Inline the type
-interface WeatherUIProps extends ToolUIProps<unknown, WeatherOutput> {} - /** * WeatherCard - Main weather display component * * This React component renders the weather data in a card layout. * When used with the React renderer, it's server-rendered to HTML. */ -export function WeatherCard({ output, helpers }: WeatherUIProps) { +export function WeatherCard({ output, helpers }: ToolUIProps<unknown, WeatherOutput>) {Option 2: Add JSDoc to clarify intent
+/** + * Props for the WeatherCard component. + * Type alias for readability. + */ interface WeatherUIProps extends ToolUIProps<unknown, WeatherOutput> {}
46-55: Consider adding badge styling for all weather conditions.The
getBadgeClassfunction only handlessunnyandrainyconditions, while theiconMapdefines 7+ weather conditions. All other conditions fall through to gray styling.Apply this diff to add styling for all defined weather conditions:
const getBadgeClass = () => { switch (output.conditions) { case 'sunny': return 'bg-green-100 text-green-800'; case 'rainy': return 'bg-blue-100 text-blue-800'; + case 'cloudy': + return 'bg-gray-300 text-gray-700'; + case 'snowy': + return 'bg-blue-200 text-blue-900'; + case 'stormy': + return 'bg-purple-100 text-purple-800'; + case 'windy': + return 'bg-cyan-100 text-cyan-800'; + case 'foggy': + return 'bg-slate-200 text-slate-700'; default: return 'bg-gray-100 text-gray-800'; } };
86-86: Consider making wind speed units dynamic.The wind speed units are hardcoded as "km/h", while temperature units are dynamic (°C/°F). This creates an inconsistency—users in regions using Fahrenheit typically expect wind speed in mph.
You could add a
windSpeedUnitsfield toWeatherOutputor derive it from the temperature units:- <div className="text-lg font-semibold text-gray-800 dark:text-gray-100">{output.windSpeed} km/h</div> + <div className="text-lg font-semibold text-gray-800 dark:text-gray-100"> + {output.windSpeed} {output.units === 'celsius' ? 'km/h' : 'mph'} + </div>libs/sdk/src/tool/flows/call-tool.flow.ts (1)
359-455: Finalize stage: UI rendering + platform detection logic looks solidThe new finalize logic cleanly validates
tool/rawOutput, usessafeParseOutput, and then conditionally renders UI when a config is present. Platform detection viaauthInfo.sessionIdPayload.platformType→scope.notifications.getPlatformType(sessionId)→'openai'is robust, and UI rendering is fully wrapped in atry/catchso tool calls don’t fail if UI generation does. MerginguiResult.metaintoresult._metaand clearingresult.contentwhile keepingstructuredContentis a sensible contract for UI-driven tools. Consider adding focused tests for:
- tools with and without UI configs,
- different platformType sources (auth vs notifications vs default),
- and UI rendering failures to assert they don’t impact the tool result.
libs/sdk/src/common/types/options/logging.options.ts (1)
19-25: PreferLogLevel.Verboseas the canonical key inLogLevelNameSince
VERBOSEis now deprecated in favor ofVerbose, consider updating the name map to use the new member as the canonical key for clarity:export const LogLevelName: Record<LogLevel, string> = { [LogLevel.Debug]: 'debug', - [LogLevel.VERBOSE]: 'verbose', + [LogLevel.Verbose]: 'verbose', [LogLevel.Info]: 'info', [LogLevel.Warn]: 'warn', [LogLevel.Error]: 'error', [LogLevel.Off]: 'off', };Functionally this is identical (both members share the same value), but it better reflects the new preferred spelling.
docs/draft/docs/ui/components/alert.mdx (1)
74-89: Add an explicit trusted‑HTML warning for customiconcontentHere
iconis a raw HTML string (typically SVG). Unlike the Button docs, there’s no explicit note that this must not contain user input.Consider adding a short warning block (similar to the Button page) that:
- Clarifies
iconshould only be used with trusted, static icon markup.- Reminds readers that user content must go through escaping utilities/components.
This keeps the examples aligned with your XSS‑hardening guidance.
docs/draft/docs/ui/templates/custom.mdx (1)
346-348: AvoidanyingetNestedValueexample; preferunknownand a small helperThe
getNestedValuehelper usesanyfor both the input and return type:function getNestedValue(obj: any, path: string): any { return path.split('.').reduce((o, k) => o?.[k], obj); }Since your TS guidelines emphasize strict typing and avoiding
any, consider updating the example to something like:function getNestedValue(obj: unknown, path: string): unknown { if (obj === null || obj === undefined) return undefined; return path.split('.').reduce<unknown>((current, key) => { if (current && typeof current === 'object') { return (current as Record<string, unknown>)[key]; } return undefined; }, obj); }This keeps the docs aligned with your strict TypeScript practices while still being easy to copy‑paste.
libs/ui/src/base-template/polyfills.ts (1)
108-131: Consider a more robust request ID generator.Using
Date.now()for JSON-RPC request IDs works but could produce duplicates under rapid sequential calls. For this client-side polyfill context, it's acceptable, but a counter or UUID would be more robust.+ var __frontmcpRequestId = 0; + window.__frontmcp.callTool = async function(toolName, args) { // ... body: JSON.stringify({ jsonrpc: '2.0', method: 'tools/call', params: { name: toolName, arguments: args || {} }, - id: Date.now() + id: ++__frontmcpRequestId })libs/sdk/src/tool/ui/render-template.ts (1)
86-90: Consider using a shared HTML escaping utility.The manual escaping is missing double-quote (
") and single-quote (') escaping. WhilemdxContentis typically developer-provided template code, using a consistentescapeHtmlutility would be more robust and aligned with the codebase pattern.- const escaped = mdxContent.replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>'); + // Import or inline a proper escapeHtml utility + const escaped = mdxContent + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''');apps/ui/react-demo/src/main.ts (1)
1-28: React demo server wiring looks correctDecorator config (apps list, logging, HTTP port, transparent auth with remote provider) is coherent and matches the MDX demo pattern. For anything beyond a demo, consider moving the port and remote provider URL to env/config rather than hard-coding them.
apps/ui/react-demo/tsconfig.app.json (1)
1-25: TS config is suitable for React + decorators; ensure base stays strictOptions here (decorators, emit metadata, react-jsx, build info) look appropriate for this demo app and align with the decorator usage. Just make sure the extended base tsconfig keeps
strict/noImplicitAnyenabled so the app stays within the project’s strict TypeScript guidelines.apps/ui/mdx-demo/webpack.config.js (1)
1-28: Webpack + Nx setup is sound; consider env-driven mode/devtool and verify output pathThe NxAppWebpackPlugin wiring (Node target, tsc compiler, main, tsconfig, externals) looks correct for this MDX server demo. Two small follow‑ups:
- Drive
mode/devtoolfromNODE_ENVso production builds don’t always usedevelopment+eval-cheap-module-source-map.- Double‑check that
../../../dist/apps/mdx-demomatches your intended dist layout for this project (and is consistent with the React demo’s webpack config).For example, you could make
mode/devtoolenvironment‑aware:- module.exports = { - output: { +const isProd = process.env.NODE_ENV === 'production'; + +module.exports = { + output: { path: join(__dirname, '../../../dist/apps/mdx-demo'), - ...(process.env.NODE_ENV !== 'production' && { + ...(!isProd && { devtoolModuleFilenameTemplate: '[absolute-resource-path]', }), }, - mode: 'development', - devtool: 'eval-cheap-module-source-map', + mode: isProd ? 'production' : 'development', + devtool: isProd ? 'source-map' : 'eval-cheap-module-source-map',docs/draft/docs/ui/advanced/mcp-bridge.mdx (2)
23-24: Alignprovidertype with documented Gemini supportThe
MCPBridge.providerunion only lists'openai' | 'ext-apps' | 'claude' | 'unknown', but the platform compatibility table below includes Gemini.If Gemini has a dedicated provider id at runtime, it would be clearer to add it to the union. If Gemini is surfaced as
ext-appsorunknown, consider calling that out explicitly in the table or a short note so the types and docs stay in sync.Also applies to: 292-301
145-148: Clarify wheresupportsFullInteractivity()comes from or reference the actual helperThe warning recommends checking
supportsFullInteractivity(), but this function isn’t introduced anywhere in the doc or interface. Elsewhere in the codebase you likely expose helpers likeisMCPBridgeSupported()or similar.Consider either:
- Showing an import/usage snippet for the real helper you expect people to call, or
- Rewording to “Check for
window.mcpBridge/capabilities (e.g.window.mcpBridge?.setWidgetState) before relying on this feature.”Right now this reads like a magic function.
apps/ui/react-demo/webpack.config.js (1)
5-13: Derivemode(and possiblydevtool) fromNODE_ENVfor correct prod builds
modeis hard‑coded to'development'anddevtoolto'eval-cheap-module-source-map', while the Nx build target only passes--node-env=production/developmentto webpack-cli. That means “production” builds will still run with development mode and dev tooling.Consider something like:
const isProd = process.env.NODE_ENV === 'production'; module.exports = { mode: isProd ? 'production' : 'development', devtool: isProd ? 'source-map' : 'eval-cheap-module-source-map', // ... };so prod builds get the expected optimizations and dev builds keep fast source maps.
apps/ui/react-demo/project.json (1)
4-37: Verifyserveworks with this custom webpack build wiringHere
buildis anx:run-commandswrapper aroundwebpack-cli build, whileserveuses@nx/js:nodewithbuildTarget: "build". Because the build target doesn’t expose anoutputPath/mainin its options, the node executor may not know where the built entrypoint lives.Two suggestions:
- Double‑check
nx serve react-demoactually starts the built bundle and reloads on changes.- If it’s flaky, consider switching
buildto an Nx webpack executor (e.g.@nx/webpack:webpackor similar) with an explicitoutputPathandmain, and haveserve.buildTargetpoint at that ("react-demo:build:development").That will give you better cacheability and a more standard Nx pipeline.
docs/draft/docs/ui/theming/dark-mode.mdx (1)
16-22: Clarify whetherthemeis onmcpBridgeitself or only undercontextThe “How Dark Mode Works” snippet shows both:
window.mcpBridge.theme // 'light' | 'dark' window.mcpBridge.context.theme // 'light' | 'dark' | 'system'But the MCP Bridge interface earlier in the docs only exposes
context.theme. To avoid confusion, it would help to:
- Either document that
themeis a top‑level shortcut officially supported, or- Remove the top‑level
themeaccess here and just usewindow.mcpBridge.context.theme.Right now it’s ambiguous which is canonical.
docs/draft/docs/ui/templates/react.mdx (1)
10-11: Clarify hook usage vs hydration and link to the hydration docsThe intro says React templates “support full React features including hooks,” while the best‑practices section later says “Don’t use hooks for SSR – they won’t work without hydration.” That’s true, but a bit ambiguous without explicitly tying it to hydration.
Suggestion:
- In this page, call out that stateful hooks (
useState,useEffect, etc.) requirehydrate: truein the UI config, and- Add a short “For interactive widgets, see React Hydration” link near the hooks guidance.
That will make it clearer when hooks are safe to use and how to enable them.
Also applies to: 352-377
libs/ui/src/renderers/html.renderer.ts (1)
17-20: Consider reusing existing type from runtime/types.The
TemplateBuilderFntype is defined locally, but a matching type already exists inlibs/ui/src/runtime/types.ts(line 76). Consider importing and reusing the existing type to avoid duplication.Apply this diff:
-import type { TemplateContext } from '../runtime/types'; +import type { TemplateContext, TemplateBuilderFn } from '../runtime/types'; import type { PlatformCapabilities } from '../theme'; import type { UIRenderer, TranspileResult, TranspileOptions, RenderOptions, RuntimeScripts } from './types'; import { isTemplateBuilderFunction } from './utils/detect'; import { hashString } from './utils/hash'; -/** - * Template builder function type. - */ -type TemplateBuilderFn<In, Out> = (ctx: TemplateContext<In, Out>) => string; - /** * Types this renderer can handle. */libs/ui/src/renderers/registry.test.ts (1)
217-227: Consider usingobjectfor generic defaults in test helpers.The
createContexthelper uses{}as the default type forInandOutparameters. While this works for test code, usingobjectwould be more semantically correct and align with TypeScript best practices for "any object" types.Apply this diff:
- const createContext = <In = {}, Out = {}>(input: In, output: Out): TemplateContext<In, Out> => ({ + const createContext = <In = object, Out = object>(input: In, output: Out): TemplateContext<In, Out> => ({ input, output, helpers: {Note: This is a test helper, so the current implementation is acceptable. The change would primarily address the ESLint warning.
Also applies to: 257-267
libs/ui/src/base-template/bridge.ts (1)
112-117: Shallow equality check may miss mutations.The comparison
state.data === datauses reference equality. If the same object reference is mutated and re-passed, the change won't be detected. This is acceptable if data is always replaced (immutable pattern), but worth documenting.Consider documenting this expected usage pattern in the JSDoc, or using a version/timestamp approach if mutation detection is needed:
function setData(data) { - // Skip if data hasn't actually changed (prevents unnecessary re-renders) + // Skip if data reference hasn't changed (assumes immutable data pattern) if (!state.loading && state.data === data) return; state = { data: data, loading: false, error: null }; notify(); }libs/ui/src/renderers/registry.ts (2)
137-148: Consider using specific error class per coding guidelines.The error thrown at line 140 uses a generic
Error. Per coding guidelines, specific error classes with MCP error codes should be used instead of generic errors. This same pattern appears at lines 207 and 243.If an MCP error class exists for configuration/registry errors:
if (!fallback) { - throw new Error(`Default renderer '${this.defaultRenderer}' not found`); + throw new RegistryError(`Default renderer '${this.defaultRenderer}' not found`, { code: 'RENDERER_NOT_FOUND' }); }
119-135: Confidence normalization assumes priority range of 0-100.The calculation
renderer.priority / 100at line 125 normalizes confidence to 0-1, but this assumes priorities are in the 0-100 range. If a renderer has priority > 100, confidence would exceed 1.0. Consider clamping or documenting the expected priority range.const result: DetectionResult = { renderer, - confidence: renderer.priority / 100, // Normalize to 0-1 + confidence: Math.min(renderer.priority / 100, 1), // Normalize to 0-1, clamped reason: `Matched by ${renderer.type} renderer`, };libs/ui/src/renderers/react.renderer.ts (1)
166-182: Type-unsafe cache storage pattern.The cache stores a function as
code: Component as unknown as stringand retrieves it with the inverse cast. This works at runtime but violates type safety. Consider using theComponentCacheclass from cache.ts for this purpose, which is designed for arbitrary component types.- // Check cache for the executed component - const cached = transpileCache.getByKey(`exec:${transpiled.hash}`); - if (cached) { - Component = cached.code as unknown as typeof Component; - } else { - // Execute the transpiled code to get the component - Component = await executeTranspiledCode(transpiled.code, { - // Provide any additional MDX components if specified - ...options?.mdxComponents, - }); - - // Cache the component function - transpileCache.setByKey(`exec:${transpiled.hash}`, { - code: Component as unknown as string, - hash: transpiled.hash, - cached: false, - }); - } + // Check cache for the executed component + const cacheKey = `exec:${transpiled.hash}`; + const cached = componentCache.get(cacheKey); + if (cached) { + Component = cached as typeof Component; + } else { + // Execute the transpiled code to get the component + Component = await executeTranspiledCode(transpiled.code, { + // Provide any additional MDX components if specified + ...options?.mdxComponents, + }); + + // Cache the component function + componentCache.set(cacheKey, Component); + }You would also need to import
componentCacheat line 24:import { transpileCache, componentCache } from './cache';apps/ui/mdx-demo/src/apps/weather/tools/get-weather.tool.tsx (2)
117-117: Use proper type instead ofanyfor helpers.The
helpers: anytype violates coding guidelines. Use the appropriate helper type from the SDK.-const mdxTemplateFunction = (ctx: { output: WeatherOutput; helpers: any }) => { +import type { TemplateHelpers } from '@frontmcp/ui'; + +const mdxTemplateFunction = (ctx: { output: WeatherOutput; helpers: TemplateHelpers }) => {
95-114: UnusedmdxTemplatevariable - consider removing or using it.The
mdxTemplateconstant is defined but not used. The tool usesmdxTemplateFunctioninstead. If this is intentional as a documentation example, add a comment; otherwise, remove dead code.If keeping as an example:
+// Example: Static MDX template string (not used - see mdxTemplateFunction below) const mdxTemplate = `Or remove if not needed.
docs/draft/docs/ui/advanced/custom-renderers.mdx (1)
232-234: Documentation example usesanytype - consider modeling best practices.Since documentation often serves as a template for users, consider using proper types in examples.
-function getNestedValue(obj: any, path: string): any { +function getNestedValue(obj: Record<string, unknown>, path: string): unknown { return path.split('.').reduce((o, k) => o?.[k], obj); }libs/sdk/src/tool/ui/tool-ui.registry.ts (1)
294-311:getLatestForToolhas O(n) complexity on cache size.This method iterates the entire cache to find the latest entry for a tool. With
MAX_CACHE_SIZE = 1000, this could become a performance concern under high load or frequent lookups.Consider maintaining a secondary index (e.g.,
Map<toolName, uri>) for the latest entry per tool if this method is called frequently.libs/ui/src/renderers/mdx.renderer.ts (1)
36-39: CDN URLs should reference theme configuration, not be hardcoded.As per coding guidelines: "Hard-code CDN URLs only in theme presets; always reference theme.cdn configuration in component code." The
REACT_CDNconstant should be moved to a theme preset or reference the theme's CDN configuration.libs/sdk/src/common/metadata/tool-ui.metadata.ts (1)
17-263: Metadata types look solid; consider reducing duplication with UI runtime typesThe UI metadata surface here (CSP, helpers, context, serving mode,
ToolUIConfig) is well‑structured, usesunknownas the generic default, and the onlyanyusages are in places where you intentionally avoid a hard React/MDX type dependency, which is reasonable.The only concern is long‑term drift:
UIContentSecurityPolicy,TemplateHelpers,TemplateContext,WidgetServingMode, and thetemplate/mdxComponentsshapes closely mirror what’s inlibs/ui/src/runtime/types.ts. If those evolve independently, SDK metadata and UI runtime may silently diverge.If dependency boundaries allow, consider centralizing these shared UI primitives in a small common module (or adding a type‑level bridge like assignability tests) so both SDK and UI runtimes stay in lock‑step.
libs/ui/src/renderers/types.ts (1)
250-401: Avoid duplicating Tool UI config fields; deriveExtendedToolUIConfigfrom runtime types and align wrapper context
ExtendedToolUIConfigcurrently re‑declares most of the fields that already exist on the runtimeToolUIConfig(CSP, displayMode, servingMode, etc.) and also re‑encodes literal unions like'inline' | 'mcp-resource' | 'direct-url' | 'custom-url'. This makes it easy for the renderer view of config to drift fromlibs/ui/src/runtime/types.tsandlibs/sdk/src/common/metadata/tool-ui.metadata.tsif someone updates one but not the other.Additionally, the
wrappersignature here takes aWrapperContext(tool name, renderer type, platform, runtimeScripts), while the runtimeToolUIConfig.wrapperis typed to receive aTemplateContext<In, Out>in the snippets you provided. If the renderers are the ones actually invokingwrapper, you probably want a single authoritative signature; otherwise the docs/types forwrapperwill be inconsistent.To keep things in sync and make the override points explicit, you could derive
ExtendedToolUIConfigfrom the runtimeToolUIConfigand only override the fields that actually differ (template/wrapper/mdxComponents), e.g.:-import type { TemplateContext, TemplateHelpers, TemplateBuilderFn } from '../runtime/types'; +import type { + TemplateContext, + TemplateHelpers, + TemplateBuilderFn, + ToolUIConfig, +} from '../runtime/types'; @@ -export type ToolUITemplate<In = unknown, Out = unknown> = +export type ToolUITemplate<In = unknown, Out = unknown> = | TemplateBuilderFn<In, Out> // HTML template function: (ctx) => string | string // MDX or static HTML string | ReactComponentType<In, Out>; // React functional/class component @@ -export interface ExtendedToolUIConfig<In = unknown, Out = unknown> { - template: ToolUITemplate<In, Out>; - hydrate?: boolean; - wrapper?: (content: string, ctx: WrapperContext) => string; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - mdxComponents?: Record<string, (props: any) => any>; - // ...plus all the inlined csp/widgetAccessible/displayMode/etc. fields... -} +export type ExtendedToolUIConfig<In = unknown, Out = unknown> = + Omit<ToolUIConfig<In, Out>, 'template' | 'wrapper' | 'mdxComponents'> & { + template: ToolUITemplate<In, Out>; + wrapper?: (content: string, ctx: WrapperContext) => string; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mdxComponents?: Record<string, (props: any) => any>; + };This way:
- CSP, displayMode, servingMode, etc. all stay in lock‑step with the runtime
ToolUIConfig.- The only divergence is documented and intentional (wrapper context + stronger MDX component type).
- You remove duplicated literal unions and the chance of subtle mismatches later.
If you intend
wrapperto useWrapperContexteverywhere, I’d also update theToolUIConfigdefinition (and its docs) inruntime/types.tsto match, so authors only see one contract for that hook.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
apps/ui/mdx-demo/e2e/weather.e2e.ts (6)
95-103: Enhance XSS test to verify actual escaping.This test verifies that
<script>doesn't appear in the output, but doesn't confirm the malicious input is actually escaped and rendered safely. The test could pass if the location field isn't rendered at all.Consider verifying the escaped content appears in the HTML:
test('handles special characters safely (XSS prevention)', async ({ mcp }) => { const result = await mcp.tools.call('get_weather_mdx', { location: '<script>alert("xss")</script>', }); expect(result).toBeSuccessful(); expect(result).toBeXssSafe(); expect(result).toNotContainRawContent('<script>'); + + // Verify the input is escaped and rendered + const html = UIAssertions.assertRenderedUI(result); + expect(html).toContain('<script>'); // or similar escaped form });
125-139: Tighten the separator assertion logic.The assertion on lines 137-138 includes
'Powered by'which appears unrelated to Markdown separator rendering and could match footer text, making the test fragile.Consider focusing on actual separator elements:
// The template has --- which should create some separator element // Accept either <hr> or any block-level element with separator styling - const hasSeparator = html.includes('<hr') || html.includes('border-') || html.includes('Powered by'); + const hasSeparator = html.includes('<hr') || html.includes('border-') || /<div[^>]*class="[^"]*separator[^"]*"/.test(html); expect(hasSeparator).toBe(true);
112-123: Note: Test is tightly coupled to mock data.The hard-coded expectations (line 120:
'foggy', line 122:18) are tightly coupled to the mock weather service implementation. While this is acceptable for E2E tests with known fixtures, consider documenting the dependency or extracting these expectations to a test constants file for maintainability.
32-193: Consider testing across platform configurations.Based on learnings, tests should verify behavior across different platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable. The current test suite only uses one transport configuration. Consider adding test cases or variants that verify MDX rendering works consistently across different platform contexts, especially if UI rendering or delivery varies by platform.
Based on learnings, UI tests should cover platform variations where applicable.
32-193: Consider extracting common test setup pattern.Several tests repeat the same pattern: call the tool, verify success, and check rendered HTML. While this repetition aids test readability, consider extracting a helper if the pattern grows more complex:
async function callWeatherTool( mcp: MCPClient, location: string, units?: 'celsius' | 'fahrenheit' ) { const result = await mcp.tools.call('get_weather_mdx', { location, units }); expect(result).toBeSuccessful(); expect(result).toHaveRenderedHtml(); return result; }This would reduce duplication while maintaining test clarity.
32-193: Add negative test cases and edge scenarios.The test suite thoroughly covers successful rendering but lacks tests for error conditions and edge cases. Consider adding:
- Invalid/unknown location handling
- Empty or null location parameter
- Network/service errors
- Different units parameter variations ('fahrenheit' is only tested once at line 156)
- Boundary conditions (very long location names, special Unicode characters)
This would improve coverage and align with the 95%+ coverage requirement from the learnings.
Based on learnings, comprehensive test coverage including edge cases is expected.
libs/testing/src/client/mcp-test-client.ts (1)
886-932: Tool UI detection and json() behavior look solid; consider a tiny robustness tweakThe
_meta['ui/html']/structuredContenthandling cleanly differentiates Tool UI vs plain JSON responses, andhasToolUI()matches the new interface usage; behavior for non-UI tools stays intact. One optional improvement: if_meta['ui/html']is present butstructuredContentis unexpectedly missing or null, you might want to log or throw a clearer error instead of silently falling back to text parsing, to surface misconfigured Tool UI during tests.libs/testing/src/matchers/mcp-matchers.ts (1)
20-24: Good integration of UI matchers into the MCP matcher surfaceImporting
uiMatchersand spreading them intomcpMatcherscleanly exposes the UI matchers toexpect.extend(mcpMatchers)without changing existing behavior. Just make sure theMcpMatchersinterface inmatcher-types.tsis updated to declare these UI matchers as well so TypeScript IntelliSense stays in sync with the runtime surface.Also applies to: 448-474
apps/ui/react-demo/e2e/weather.e2e.ts (1)
1-20: Well-structured E2E test suite with good XSS coverage.The test file covers key UI scenarios including rendering, data binding, HTML structure, and XSS safety. Based on learnings, the XSS testing at lines 62-70 aligns with the requirement to test HTML escaping for user-provided content.
Consider importing the
WeatherOutputtype from the actual tool implementation rather than redefining it locally to avoid drift between the test interface and the actual output shape.libs/testing/src/ui/ui-matchers.ts (2)
104-106: Escape regex metacharacters intagparameter to prevent false matches.The
tagparameter is interpolated directly into the regex. If a test passes a value containing regex metacharacters (e.g.,div.*), it could match unintended elements. While catastrophic backtracking is unlikely here, escaping ensures correctness.+function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + const toContainHtmlElement: MatcherFunction<[tag: string]> = function (received, tag) { const html = extractUiHtml(received); if (!html) { return { pass: false, message: () => `Expected to find <${tag}> element, but no HTML content found`, }; } // Match opening tags: <tag> or <tag attributes> - const regex = new RegExp(`<${tag}[\\s>]`, 'i'); + const regex = new RegExp(`<${escapeRegex(tag)}[\\s>]`, 'i'); const pass = regex.test(html);
217-219: Escape regex metacharacters inclassNameparameter.Similar to
toContainHtmlElement, theclassNameis interpolated into a regex pattern. CSS class names shouldn't normally contain regex metacharacters, but defensive escaping prevents unexpected behavior in edge cases.const toHaveCssClass: MatcherFunction<[className: string]> = function (received, className) { const html = extractUiHtml(received); if (!html) { return { pass: false, message: () => `Expected HTML to have CSS class "${className}", but no HTML content found`, }; } // Match class="... className ..." or className="... className ..." - const classRegex = new RegExp(`class(?:Name)?\\s*=\\s*["'][^"']*\\b${className}\\b[^"']*["']`, 'i'); + const classRegex = new RegExp(`class(?:Name)?\\s*=\\s*["'][^"']*\\b${escapeRegex(className)}\\b[^"']*["']`, 'i'); const pass = classRegex.test(html);Add the
escapeRegexhelper function if not already added:function escapeRegex(str: string): string { return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); }libs/testing/src/ui/ui-assertions.ts (3)
140-145: Same regex escaping recommendation applies here.The
assertContainsElementfunction has the same regex interpolation issue astoContainHtmlElementin ui-matchers.ts. Apply the sameescapeRegexhelper to sanitize thetagparameter.+function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + assertContainsElement(html: string, tag: string): void { - const regex = new RegExp(`<${tag}[\\s>]`, 'i'); + const regex = new RegExp(`<${escapeRegex(tag)}[\\s>]`, 'i'); if (!regex.test(html)) { throw new Error(`Expected HTML to contain <${tag}> element`); } },
153-158: Apply regex escaping toclassNameparameter.Same issue as in ui-matchers.ts - the
classNameshould be escaped before regex interpolation.assertHasCssClass(html: string, className: string): void { - const classRegex = new RegExp(`class(?:Name)?\\s*=\\s*["'][^"']*\\b${className}\\b[^"']*["']`, 'i'); + const classRegex = new RegExp(`class(?:Name)?\\s*=\\s*["'][^"']*\\b${escapeRegex(className)}\\b[^"']*["']`, 'i'); if (!classRegex.test(html)) { throw new Error(`Expected HTML to have CSS class "${className}"`); } },
211-218: Silent catch may hide unexpected parsing failures.The empty catch block silently skips data binding validation if JSON parsing fails. While the comment explains the intent, this could mask issues where
result.text()returns malformed data unexpectedly.Consider adding structured logging or using
result.json()method fromToolResultWrapperwhich may handle Tool UI structured content more appropriately.if (boundKeys && boundKeys.length > 0) { try { - const output = JSON.parse(result.text() || '{}'); + const output = result.json<Record<string, unknown>>(); UIAssertions.assertDataBinding(html, output, boundKeys); - } catch { + } catch (e) { // If we can't parse output, skip data binding check + // This is expected for non-JSON tool outputs } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/ui/mdx-demo/e2e/weather.e2e.ts(1 hunks)apps/ui/react-demo/e2e/weather.e2e.ts(1 hunks)apps/ui/react-demo/src/main.ts(1 hunks)docs/draft/docs/servers/tools.mdx(3 hunks)docs/draft/docs/ui/integration/testing.mdx(1 hunks)libs/testing/ARCHITECTURE.md(2 hunks)libs/testing/README.md(3 hunks)libs/testing/src/client/mcp-test-client.ts(2 hunks)libs/testing/src/client/mcp-test-client.types.ts(2 hunks)libs/testing/src/expect.ts(1 hunks)libs/testing/src/index.ts(1 hunks)libs/testing/src/matchers/matcher-types.ts(1 hunks)libs/testing/src/matchers/mcp-matchers.ts(2 hunks)libs/testing/src/ui/index.ts(1 hunks)libs/testing/src/ui/ui-assertions.ts(1 hunks)libs/testing/src/ui/ui-matchers.ts(1 hunks)libs/ui/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/testing/README.md
- libs/ui/README.md
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/client/mcp-test-client.types.tslibs/testing/src/expect.tslibs/testing/src/index.tsapps/ui/react-demo/e2e/weather.e2e.tsapps/ui/react-demo/src/main.tslibs/testing/src/ui/index.tsapps/ui/mdx-demo/e2e/weather.e2e.tslibs/testing/src/matchers/mcp-matchers.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/client/mcp-test-client.tslibs/testing/src/matchers/matcher-types.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/client/mcp-test-client.types.tslibs/testing/src/expect.tslibs/testing/src/index.tslibs/testing/ARCHITECTURE.mdlibs/testing/src/ui/index.tslibs/testing/src/matchers/mcp-matchers.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/client/mcp-test-client.tslibs/testing/src/matchers/matcher-types.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.ts
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/ui/integration/testing.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/ui/integration/testing.mdx
🧠 Learnings (14)
📚 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/client/mcp-test-client.types.tslibs/testing/src/expect.tslibs/testing/src/matchers/mcp-matchers.tslibs/testing/src/client/mcp-test-client.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/expect.tslibs/testing/ARCHITECTURE.mdlibs/testing/src/matchers/mcp-matchers.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/index.tsapps/ui/react-demo/e2e/weather.e2e.tslibs/testing/ARCHITECTURE.mdlibs/testing/src/ui/index.tsapps/ui/mdx-demo/e2e/weather.e2e.tsdocs/draft/docs/ui/integration/testing.mdxlibs/testing/src/matchers/mcp-matchers.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/client/mcp-test-client.tslibs/testing/src/matchers/matcher-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/testing/src/index.tslibs/testing/src/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/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
libs/testing/src/index.tsapps/ui/react-demo/e2e/weather.e2e.tslibs/testing/ARCHITECTURE.mdlibs/testing/src/ui/index.tsapps/ui/mdx-demo/e2e/weather.e2e.tsdocs/draft/docs/ui/integration/testing.mdxlibs/testing/src/matchers/mcp-matchers.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/matchers/matcher-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/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
libs/testing/src/index.tsapps/ui/react-demo/e2e/weather.e2e.tslibs/testing/ARCHITECTURE.mdlibs/testing/src/ui/index.tsapps/ui/mdx-demo/e2e/weather.e2e.tsdocs/draft/docs/ui/integration/testing.mdxlibs/testing/src/matchers/mcp-matchers.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/matchers/matcher-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/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
libs/testing/src/index.tsapps/ui/react-demo/e2e/weather.e2e.tslibs/testing/ARCHITECTURE.mdlibs/testing/src/ui/index.tsapps/ui/mdx-demo/e2e/weather.e2e.tsdocs/draft/docs/ui/integration/testing.mdxlibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/matchers/matcher-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 : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
apps/ui/react-demo/e2e/weather.e2e.tslibs/testing/ARCHITECTURE.mddocs/draft/docs/ui/integration/testing.mdxlibs/testing/src/ui/ui-matchers.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/ARCHITECTURE.mdlibs/testing/src/ui/index.tslibs/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.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/ARCHITECTURE.mdlibs/testing/src/matchers/matcher-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 : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
libs/testing/ARCHITECTURE.md
📚 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/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/components/**/*.ts : Use pure HTML string generation without React/Vue/JSX - components return HTML strings only
Applied to files:
libs/testing/src/ui/ui-matchers.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/testing/src/ui/ui-matchers.tslibs/testing/src/ui/ui-assertions.ts
🧬 Code graph analysis (5)
apps/ui/react-demo/src/main.ts (3)
apps/ui/mdx-demo/src/main.ts (1)
FrontMcp(12-23)libs/testing/src/client/mcp-test-client.types.ts (1)
LogLevel(208-208)libs/testing/src/index.ts (1)
LogLevel(58-58)
apps/ui/mdx-demo/e2e/weather.e2e.ts (1)
libs/testing/src/ui/ui-assertions.ts (1)
UIAssertions(26-222)
libs/testing/src/matchers/mcp-matchers.ts (3)
libs/testing/src/index.ts (1)
uiMatchers(211-211)libs/testing/src/ui/index.ts (1)
uiMatchers(17-17)libs/testing/src/ui/ui-matchers.ts (1)
uiMatchers(295-304)
libs/testing/src/ui/ui-assertions.ts (1)
libs/testing/src/client/mcp-test-client.types.ts (1)
ToolResultWrapper(128-156)
libs/testing/src/client/mcp-test-client.ts (1)
libs/testing/src/assertions/mcp-assertions.ts (1)
isError(218-224)
🪛 ast-grep (0.40.0)
libs/testing/src/ui/ui-matchers.ts
[warning] 104-104: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${tag}[\\s>], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 217-217: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(class(?:Name)?\\s*=\\s*["'][^"']*\\b${className}\\b[^"']*["'], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
libs/testing/src/ui/ui-assertions.ts
[warning] 140-140: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${tag}[\\s>], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 153-153: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(class(?:Name)?\\s*=\\s*["'][^"']*\\b${className}\\b[^"']*["'], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (25)
docs/draft/docs/servers/tools.mdx (7)
264-264: Tool metadata table additions look good.The
uifield has been properly integrated into both the metadata configuration and field descriptions table. Placement is clear and the description accurately captures the purpose of UI configuration.Also applies to: 282-282
640-670: Tool UI section introduction and basic example are clear.The introductory example effectively demonstrates the core UI pattern with HTML templating, context access, and helper methods. Structure follows the doc's established pattern well.
676-704: Template types showcase is well-organized.The CodeGroup effectively shows the three template options (HTML, React, MDX) side-by-side. However, the React component example needs clarification: what props does
WeatherCardreceive? Does it receive the context object, just the output, or something else? This should be documented either here or with a code comment showing the expected signature.
723-746: @frontmcp/ui integration example is practical.The complex example effectively demonstrates composing UI components from the component library. The pattern is clear and shows real-world usage well.
748-764: Testing pattern is well-presented.The E2E testing example using
@frontmcp/testingshows clear assertions. However, verify that the assertion methods shown (toHaveRenderedHtml(),toBeXssSafe(),toContainBoundValue(), andUIAssertions.assertValidUI()) actually exist in the testing library or update if the API differs.
656-659: Document thehelpersobject API and mdxComponents pattern.The examples reference
ctx.helperswith methods likeescapeHtml()andformatCurrency(), and the MDX example showsmdxComponents: { Alert }, but neither is fully documented here:
- What other methods does
helpersprovide? (OnlyescapeHtmlandformatCurrencyare shown; are there others?)- What is the expected signature and props for custom MDX components like
Alert?Either expand the documentation here or ensure these are clearly documented in the referenced guides (e.g.,
/docs/ui/templates/overview).Also applies to: 679-679, 731-740
767-778: Verify referenced documentation pages exist or are pending.The CardGroup at the end references four documentation paths:
/docs/ui/integration/tools/docs/ui/components/overview/docs/ui/templates/overview/docs/ui/integration/testingConfirm these pages exist in the current documentation structure or if they're being added as part of this PR.
apps/ui/react-demo/src/main.ts (4)
1-7: LGTM! Clear documentation.The documentation clearly explains the purpose of this React demo and how it differs from other UI template approaches.
9-10: Imports look good.The imports follow the established pattern from the MDX demo. The LogLevel.Debug usage on line 15 matches the pattern used in other demos within this PR.
12-22: Configuration follows the established pattern correctly.The FrontMcp decorator configuration matches the pattern from the MDX demo, with appropriate port differentiation (3003 vs 3004) to avoid conflicts between running demos. The hierarchical configuration structure aligns with the coding guidelines.
23-23: Empty Server class is correct for the decorator pattern.The empty class serves as a holder for the FrontMcp decorator configuration, which is the established pattern across the demo applications.
apps/ui/mdx-demo/e2e/weather.e2e.ts (2)
10-21: LGTM! Clean imports and well-typed interface.The WeatherOutput interface follows TypeScript best practices with proper literal union types for the units field and clear, required properties.
25-30: LGTM! Proper test configuration.The test.use() configuration correctly sets up the MDX demo server with appropriate transport and public mode settings.
libs/testing/src/index.ts (1)
206-211: UI testing barrel exports fit the public API cleanlyExporting
uiMatchersandUIAssertionsfrom the top-level testing index keeps the UI testing surface discoverable and matches the documented imports (import { uiMatchers, UIAssertions } from '@frontmcp/testing';) without introducing aliases.libs/testing/src/client/mcp-test-client.types.ts (1)
139-156: ToolResultWrapper type now accurately documents and exposes Tool UI behaviorThe updated
json()documentation and the newhasToolUI()flag align with the runtime behavior inwrapToolResult, giving tests an explicit way to branch on Tool UI responses while keeping the API backwards compatible for non-UI tools.libs/testing/src/ui/index.ts (1)
1-18: UI testing barrel is straightforward and matches usage examplesThis small barrel cleanly exposes
uiMatchersandUIAssertionswith a clear example; it aligns with the main testing index re-exports and keeps the UI testing surface well-structured.libs/testing/ARCHITECTURE.md (1)
317-373: Architecture docs accurately describe the new UI testing surfaceThe new “UI Matchers (
src/ui/)" section and the updated file tree clearly documentui-matchers.ts,ui-assertions.ts, and the UI barrel, with examples that match the actual matcher and helper names. This should make the Tool UI testing story much easier to understand for contributors.Also applies to: 1231-1235
docs/draft/docs/ui/integration/testing.mdx (1)
1-401: Tool UI testing guide is comprehensive and aligned with the new APIsThis MDX page lines up well with the exported testing surface (
test,expect, UI matchers,UIAssertions,hasToolUI, and the Tool-UI-awarejson()behavior) and lives correctly underdocs/draft/docs. The examples cover React/MDX/HTML templates, XSS, and data binding in a way that matches the underlying implementation.libs/testing/src/expect.ts (1)
26-42: Extended McpExpectMatchers nicely types not/resolves/rejects with MCP matchersAugmenting
McpExpectMatchersso that.not,.resolves, and.rejectsalso carryMcpMatchersgives good typing for async and negated MCP/UI expectations (e.g.,expect(promise).resolves.toBeSuccessful(),expect(result).not.toHaveRenderedHtml()) without altering runtime behavior.apps/ui/react-demo/e2e/weather.e2e.ts (2)
105-106: Temperature comparison assumption is context-dependent.The assertion assumes positive Celsius temperatures (Fahrenheit will be higher). This holds for the New York mock data. If mock data changes to very cold temperatures (below -40°), this test would fail unexpectedly. The comment on line 105 helps clarify the intent.
117-123: Type assertion is acceptable for test utility interop.The
as unknown as Record<string, unknown>cast is needed becauseassertDataBindingexpects a generic record type. This pattern works but a future improvement could be makingassertDataBindinggeneric to avoid the double assertion.libs/testing/src/matchers/matcher-types.ts (1)
207-300: Well-documented UI matcher type declarations.The new UI matchers are properly typed with generic return type
Rand include comprehensive JSDoc examples. The documentation follows best practices as per coding guidelines for libs/**. Noanytypes are used, aligning with strict TypeScript mode requirements.libs/testing/src/ui/ui-matchers.ts (2)
28-45: Type assertions in helper functions are acceptable.The type assertions using
asat lines 34 and 51 are appropriate here since the code immediately checks forundefinedand validates the structure. This follows defensive coding patterns rather than blindly asserting.
144-171: XSS checks cover common attack vectors.The matcher checks for
<script>tags, inline event handlers (on*), andjavascript:URIs. This covers the most common XSS patterns. For a testing utility, this level of coverage is appropriate.For future consideration: additional vectors like
data:text/htmlURIs or CSSexpression()could be added, but these are less common in typical Tool UI contexts.libs/testing/src/ui/ui-assertions.ts (1)
33-58: Good defensive validation with clear error messages.The
assertRenderedUImethod properly validates the _meta structure, checks for string type, and detects mdx-fallback. Error messages are descriptive and actionable, helping developers debug issues quickly.
…and metadata validation
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
libs/testing/src/ui/ui-assertions.ts (1)
196-211: Widget metadata assertion now matches matcher behavior
assertWidgetMetadatanow checks for any ofui/html,openai/outputTemplate, orui/mimeTypeand throws a message listing those keys when none are present. This brings it in line withtoHaveWidgetMetadatainui-matchers.tsand resolves the earlier inconsistency between assertion and matcher APIs.
🧹 Nitpick comments (18)
docs/draft/docs/ui/advanced/mcp-bridge.mdx (2)
113-113: Minor: Hyphenate compound adjectives for consistency.Line 113 reads: "This uses the platform's native link opening mechanism (new tab, in-app browser, etc.)."
For consistency with hyphenation conventions, consider: "new-tab, in-app-browser" as a list of compound adjectives.
316-316: Minor: Strengthen writing with synonym choice.Line 316 reads: "3. Show loading states - Give feedback during async operations"
Consider: "Provide feedback during async operations" (slightly more formal/direct).
docs/draft/docs/ui/layouts/templates.mdx (1)
1-369: Documentation examples are well-structured and follow consistent patterns.The file provides clear TypeScript interfaces for each layout type, practical usage examples, and a comprehensive custom layout walkthrough. All code examples use either controlled data (internal state, configuration) or properly escaped user-provided content (productLayout).
A few notes for clarity (optional improvements):
- Line 109:
avatar({ name: user.name, size: 'sm' })— if avatar renders user.name into markup, verify thatavatar()component already escapes its input, or document the expectation.- Line 325:
${product.price.toFixed(2)}— numeric formatting is safe, but consider the pattern: if readers adapt this for other number-like fields, escaping defensive values could be helpful context.As a documentation-only addition in the draft folder, this is well-suited for this location per coding guidelines. Recommend keeping it here rather than docs/docs/** until reviewed by maintainers.
If planning a followup, consider a brief callout in the "Best Practices" section (after line 363) about security:
- Emphasize that user-provided content should always be escaped before interpolation
- Link to or reference the
escapeHtml()utility as a best practicedocs/draft/docs/ui/advanced/custom-renderers.mdx (1)
263-301: Security warning is now present, but consider adding a safe implementation example.Good catch on the previous review—the security warning is now prominent (lines 263-265) and clearly explains the danger. However, to make this documentation maximally useful and prevent copy-paste of the unsafe pattern, consider adding a code block showing the safe alternative (property-path-only interpolation) that you recommend.
For example, you could add a "Safe Alternative" section after the warning that demonstrates:
// Safe version: only property access (no expressions) const interpolate = (str: string): string => { return str.replace(/\{([a-zA-Z_.]+)\}/g, (_, path) => { const value = getNestedValue(context, path); return escapeHtml(String(value ?? '')); }); };This mirrors your YAML renderer approach (lines 189–194) and gives readers a proven, production-ready pattern to use instead of the unsafe one.
If you'd like, I can help draft this safe alternative section. Would you like me to generate that addition?
libs/sdk/src/common/decorators/tool.decorator.ts (1)
186-190: Add justification foranyor use a more specific React type.Per coding guidelines,
anytypes require strong justification. The eslint-disable comment suppresses the lint error but doesn't document whyanyis necessary here.Consider either:
- Adding inline justification explaining the variance issues with React component types
- Using a more constrained type if possible
type __UITemplateType = | ((ctx: { input: unknown; output: unknown; structuredContent?: unknown; helpers: TemplateHelpers }) => string) | string - // eslint-disable-next-line @typescript-eslint/no-explicit-any - | ((props: any) => any); // React component signature + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- React components have contravariant props and covariant returns; unknown is too restrictive for consumer DX + | ((props: any) => any);apps/ui/mdx-demo/e2e/weather.e2e.ts (2)
32-193: Add validation tests for invalid inputs and edge cases.The test suite covers happy paths comprehensively but lacks validation tests for error scenarios. Based on learnings, tests should cover invalid variant/options, unknown properties, and edge cases.
Consider adding tests for:
- Missing required
locationparameter- Invalid
unitsvalue (not'celsius'or'fahrenheit')- Unknown/extra properties in the input
- Empty string location
- Tool behavior when rendering fails gracefully
Example:
test('handles missing location parameter gracefully', async ({ mcp }) => { const result = await mcp.tools.call('get_weather_mdx', {}); // Should either fail with clear error or handle gracefully // Add appropriate assertions based on expected behavior }); test('handles invalid units value', async ({ mcp }) => { const result = await mcp.tools.call('get_weather_mdx', { location: 'Paris', units: 'kelvin' // Invalid value }); // Verify error handling or fallback behavior });Based on learnings, validation tests are expected to ensure robust error handling.
25-30: Consider testing across platform configurations.Based on learnings, tests should verify behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable. The current test suite uses a fixed configuration and doesn't exercise platform-specific rendering behavior.
If MDX rendering or UI metadata differs across platforms, consider adding platform-specific test scenarios:
test.describe('MDX Weather across platforms', () => { test('renders correctly with OpenAI platform', async ({ mcp }) => { // Test with OpenAI-specific configuration if applicable }); test('renders correctly with Claude platform', async ({ mcp }) => { // Test with Claude-specific configuration if applicable }); });If MDX rendering is platform-agnostic, this suggestion can be disregarded.
Based on learnings, platform configuration testing is expected where applicable.
libs/testing/src/ui/ui-matchers.ts (1)
102-122: Regex from variable input is acceptable here; escapeRegex mitigates risksStatic analysis will flag
new RegExp(...)built fromtag/className, but in this context:
- Both values are escaped via
escapeRegex, so they cannot inject arbitrary patterns.- The surrounding patterns are simple (no nested quantifiers), so catastrophic backtracking/ReDoS risk is very low even for long HTML strings.
- These helpers are used in test code, not in production request paths.
No change is strictly necessary. If you ever allow untrusted, unconstrained values to flow into
tag/classNamefrom external inputs, you may optionally cap their length (e.g., ignore values over some reasonable limit) to harden further, but that’s not required for this testing utility.Also applies to: 216-236
libs/testing/src/ui/ui-assertions.ts (3)
26-28: Shared escapeRegex implementation could be centralized (optional)This
escapeRegexhelper is identical to the one inui-matchers.ts. Keeping two copies is fine for now, but if you add more regex-based helpers later it may be worth extracting a small shared utility underlibs/testing/src/ui/to ensure they stay in sync.
79-99: Data-binding assertion is reasonable for primitive outputs; consider guarding complex values
assertDataBindingworks well whenoutput[key]is a primitive (string/number/boolean), butString(value)will turn objects/arrays into[object Object], which almost certainly won’t match the HTML. If you expect complex structures, you might later want to:
- Either skip non-primitive values explicitly, or
- Serialize them with
JSON.stringifyand assert against known serialized fragments.For now, assuming the bound keys are primitive is acceptable, especially since this is a test helper.
220-241: Prefer usingresult.json()for data binding in assertValidUIIn
assertValidUI, the data-binding step currently does:const output = JSON.parse(result.text() || '{}'); UIAssertions.assertDataBinding(html, output, boundKeys);A couple of improvements are possible:
ToolResultWrapperalready exposesjson<T>(), which knows how to obtain structured output for Tool UI results; using it is more robust than manually parsingtext().- Swallowing all parsing errors and silently skipping data-binding can hide real issues when callers explicitly passed
boundKeys.You could at least rely on
json()to centralize parsing/structured-content logic, while keeping the best-effort behavior if you want:- if (boundKeys && boundKeys.length > 0) { - try { - const output = JSON.parse(result.text() || '{}'); - UIAssertions.assertDataBinding(html, output, boundKeys); - } catch { - // If we can't parse output, skip data binding check - } - } + if (boundKeys && boundKeys.length > 0) { + try { + const output = result.json<Record<string, unknown>>(); + UIAssertions.assertDataBinding(html, output, boundKeys); + } catch { + // If we can't get structured output, skip data binding check + // (callers who require strict binding can call assertDataBinding themselves) + } + }This keeps
assertValidUIforgiving by default but avoids duplicating JSON-parsing logic and better matches howToolResultWrapperis intended to be used.libs/sdk/src/scope/flows/http.request.flow.ts (7)
23-33: Stage plan wiring is coherent; consider finalize behavior on hard errorsThe new
traceRequestplacement at the top ofpreand the explicitfinalizestage infinalizelook good. Witherror: [], finalize won’t run if the flow engine aborts on non‑FlowControl errors; if you want per‑request completion logging/metrics even on hard failures, consider adding'finalize'to theerrorplan as well.Also applies to: 42-53
55-59: State schema requiresdecisionbut router never populates it
httpRequestStateSchemadeclares a requireddecision, but the flow only ever writesverifyResultandintentintostate. That can confuse consumers ofthis.state.requiredand meansdecisionis “declared” but effectively unused, whilelogErroronly logsintent/verifyResult.If you want to keep
decisionin the schema, consider storing it when computed:- const transport = this.scope.auth.transport; - const decision = decideIntent(request, { ...transport, tolerateMissingAccept: true }); + const transport = this.scope.auth.transport; + const decision = decideIntent(request, { ...transport, tolerateMissingAccept: true }); + this.state.set('decision', decision);Optionally you could also include
decisionin thelogErrorstate snapshot. Alternatively, makedecisionoptional in the schema if you don’t intend to persist it.Also applies to: 178-215, 451-454
88-89: Request tracing and finalize logging look solid; only minor refinementsCapturing
requestStartTime/requestId, logging truncateduser-agent/sessionId, and emitting a final duration + intent log is a nice improvement and should be low‑risk.Two optional tweaks you might consider:
- Prefer an upstream request id header (e.g.
x-request-id) when present, falling back to the generatedreq-<ts>-<rand>id, to ease cross‑service correlation.- Reuse the same timestamp used for
requestStartTimewhen buildingrequestIdinstead of a secondDate.now()call (tiny perf/consistency nit).Also applies to: 104-128, 393-406
130-151: checkAuthorization flow is sound; consider a more specific error typeWrapping
session:verifyin try/catch, logging only non‑FlowControl errors, and persistingverifyResultinto state all look correct.The one thing to revisit is:
throw new Error('Session verification failed');Given the guideline to use specific error classes with MCP error codes, it would be better to throw a project‑specific error (e.g. a dedicated session‑verification error type) so higher layers can classify and handle it more precisely instead of seeing a generic
Error. As per coding guidelines, prefer specific error classes over bareError.
155-235: Router logic looks consistent; watch rawInput mutation and token placementThe router’s behavior—special‑casing
DELETE, preferringsession.payload.protocolwhen present, checking for terminated sessions and returning 404, and falling back todecideIntentwith clear handling ofunknownvs specific intents—reads correctly and should preserve/clarify the existing behavior.Two small points to consider:
- You’re mutating the request object via
request[ServerRequestTokens.auth]andrequest[ServerRequestTokens.sessionId]. IfServerRequestTokensis the sanctioned mechanism for attaching metadata to the request, this is fine; otherwise, it slightly conflicts with the “don’t mutate rawInput in flows; use state.set() instead” guidance. If feasible, consider flowingauthorization/sessionIdviastateor a derived context instead of mutating the raw request. As per coding guidelines, flows are generally expected not to mutate raw input.- The unauthorized branch assumes
verifyResult.prmMetadataHeaderis always present whenkind !== 'authorized'. If there are non‑authorized states without that header, this could produce an undefined header value; worth double‑checking theSessionVerifyOutputcontract.
349-391: Delete‑session behavior is clearer; consider masking sessionId in logsThe updated
handleDeleteSession:
- Validates the presence and type of
sessionId.- Uses
terminateSessionto both unregister and mark the ID as terminated.- Returns 404 when
wasRegisteredis false, while still preventing future use of that ID.- Returns
noContent()on successful deletion.That aligns well with the MCP spec comments.
From a logging/privacy standpoint, you currently log the full
sessionId:this.logger.info(`[${this.requestId}] DELETE session: ${sessionId}`); ... this.logger.warn(`[${this.requestId}] Session not found for DELETE: ${sessionId}`); ... this.logger.info(`[${this.requestId}] Session terminated: ${sessionId}`);Everywhere else you truncate session IDs (e.g.
slice(0, 20)), so for consistency and to reduce PII leakage in logs, I’d recommend truncating here as well, e.g.:- this.logger.info(`[${this.requestId}] DELETE session: ${sessionId}`); + this.logger.info(`[${this.requestId}] DELETE session: ${sessionId.slice(0, 20)}...`);(and similarly for the other two log lines).
18-19: Type catch clauses asunknownto avoid implicitanyin strict mode and align with logErrorThe
logErrormethod correctly useserror: unknown, but the catch clauses across the stages remain untyped, which defaults to implicitanydespite strict mode being enabled. Type the catch variable asunknownto maintain strict typing while supporting theinstanceof FlowControlcheck. For example:- } catch (error) { + } catch (error: unknown) { // FlowControl is expected control flow, not an error if (!(error instanceof FlowControl)) { this.logError(error, 'checkAuthorization'); } throw error; }Apply the same pattern to all catch blocks at lines 144, 236, 259, 282, 305, 340, and 384.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/ui/mdx-demo/e2e/weather.e2e.ts(1 hunks)apps/ui/mdx-demo/tsconfig.app.json(1 hunks)docs/draft/docs/ui/advanced/custom-renderers.mdx(1 hunks)docs/draft/docs/ui/advanced/mcp-bridge.mdx(1 hunks)docs/draft/docs/ui/layouts/templates.mdx(1 hunks)docs/draft/docs/ui/templates/react.mdx(1 hunks)libs/sdk/src/common/decorators/tool.decorator.ts(2 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(9 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(4 hunks)libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts(1 hunks)libs/testing/src/ui/ui-assertions.ts(1 hunks)libs/testing/src/ui/ui-matchers.ts(1 hunks)libs/ui/src/base-template/default-base-template.ts(1 hunks)libs/ui/src/renderers/html.renderer.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/ui/src/renderers/html.renderer.test.ts
- docs/draft/docs/ui/templates/react.mdx
- apps/ui/mdx-demo/tsconfig.app.json
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/transport/mcp-handlers/call-tool-request.handler.tsapps/ui/mdx-demo/e2e/weather.e2e.tslibs/testing/src/ui/ui-assertions.tslibs/ui/src/base-template/default-base-template.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/testing/src/ui/ui-matchers.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/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/common/decorators/tool.decorator.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/testing/src/ui/ui-assertions.tslibs/ui/src/base-template/default-base-template.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/testing/src/ui/ui-matchers.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/base-template/default-base-template.ts
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/ui/advanced/custom-renderers.mdxdocs/draft/docs/ui/layouts/templates.mdxdocs/draft/docs/ui/advanced/mcp-bridge.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/ui/advanced/custom-renderers.mdxdocs/draft/docs/ui/layouts/templates.mdxdocs/draft/docs/ui/advanced/mcp-bridge.mdx
🧠 Learnings (15)
📚 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/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/common/decorators/tool.decorator.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/transport/mcp-handlers/call-tool-request.handler.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/ui/mdx-demo/e2e/weather.e2e.tslibs/testing/src/ui/ui-assertions.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/testing/src/ui/ui-matchers.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/ui/mdx-demo/e2e/weather.e2e.tslibs/testing/src/ui/ui-assertions.tslibs/ui/src/base-template/default-base-template.tsdocs/draft/docs/ui/layouts/templates.mdxdocs/draft/docs/ui/advanced/mcp-bridge.mdxlibs/testing/src/ui/ui-matchers.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/ui/mdx-demo/e2e/weather.e2e.tslibs/testing/src/ui/ui-assertions.tslibs/testing/src/ui/ui-matchers.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/testing/src/ui/ui-assertions.tslibs/testing/src/ui/ui-matchers.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/testing/src/ui/ui-assertions.tslibs/ui/src/base-template/default-base-template.tsdocs/draft/docs/ui/advanced/custom-renderers.mdxlibs/sdk/src/common/decorators/tool.decorator.tsdocs/draft/docs/ui/layouts/templates.mdxdocs/draft/docs/ui/advanced/mcp-bridge.mdxlibs/testing/src/ui/ui-matchers.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/common/decorators/tool.decorator.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/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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 : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
libs/testing/src/ui/ui-matchers.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/testing/src/ui/ui-matchers.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/ui/ui-matchers.ts
🧬 Code graph analysis (5)
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (2)
libs/sdk/src/transport/mcp-handlers/mcp-handlers.types.ts (2)
McpHandlerOptions(43-46)McpHandler(26-41)libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowControl(17-41)
apps/ui/mdx-demo/e2e/weather.e2e.ts (1)
libs/testing/src/ui/ui-assertions.ts (1)
UIAssertions(38-242)
libs/ui/src/base-template/default-base-template.ts (4)
libs/ui/src/base-template/index.ts (8)
BaseTemplateOptions(16-16)McpSession(28-28)createDefaultBaseTemplate(14-14)ThemeStylesOptions(24-24)renderThemeStyles(21-21)renderMcpSessionPolyfill(28-28)renderBridgeScript(31-31)createMinimalBaseTemplate(15-15)libs/ui/src/base-template/polyfills.ts (2)
McpSession(13-18)renderMcpSessionPolyfill(32-178)libs/ui/src/base-template/theme-styles.ts (2)
ThemeStylesOptions(23-36)renderThemeStyles(66-101)libs/ui/src/base-template/bridge.ts (1)
renderBridgeScript(88-291)
libs/sdk/src/scope/flows/http.request.flow.ts (3)
libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowControl(17-41)libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
decideIntent(397-451)libs/sdk/src/common/schemas/http-output.schema.ts (1)
httpRespond(256-338)
libs/testing/src/ui/ui-matchers.ts (1)
libs/testing/src/client/mcp-test-client.types.ts (1)
ToolResultWrapper(128-156)
🪛 ast-grep (0.40.0)
libs/testing/src/ui/ui-assertions.ts
[warning] 153-153: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${escapeRegex(tag)}[\\s>], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 167-167: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(class(?:Name)?\\s*=\\s*["'][^"']*\\b${escapeRegex(className)}\\b[^"']*["'], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
libs/testing/src/ui/ui-matchers.ts
[warning] 113-113: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${escapeRegex(tag)}[\\s>], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 227-227: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(class(?:Name)?\\s*=\\s*["'][^"']*\\b${escapeRegex(className)}\\b[^"']*["'], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 LanguageTool
docs/draft/docs/ui/advanced/mcp-bridge.mdx
[grammar] ~113-~113: Use a hyphen to join words.
Context: ...`` This uses the platform's native link opening mechanism (new tab, in-app brows...
(QB_NEW_EN_HYPHEN)
[style] ~316-~316: Try using a synonym here to strengthen your writing.
Context: ...s can fail 3. Show loading states - Give feedback during async operations 4. **V...
(GIVE_PROVIDE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (26)
docs/draft/docs/ui/advanced/mcp-bridge.mdx (1)
206-216: ✓ XSS vulnerability fixed via JSON.stringify.The
todo.idinjection at line 210 now usesJSON.stringify(todo.id)instead of bare interpolation, which safely serializes the ID as a valid JS string literal. Combined withhelpers.escapeHtml(todo.text)at line 212, the widget example properly mitigates injection/XSS risks. This resolves the previous review concern about unsafe ID interpolation.docs/draft/docs/ui/layouts/templates.mdx (1)
276-345: Security concern from previous review has been properly addressed.The
productLayoutexample now correctly appliesescapeHtml()to all user-provided content before interpolation:
- Product name and description are escaped (lines 296-297)
- Image URLs are escaped for both main and thumbnail galleries (lines 308, 312)
- Escaped values are used in HTML attributes and content (lines 322-323)
This aligns with the learnings about preventing XSS vulnerabilities in the UI library and demonstrates safe usage patterns for readers who reuse this example.
docs/draft/docs/ui/advanced/custom-renderers.mdx (3)
89-123: Custom renderer example correctly escapes output.The
myCustomRendererimplementation properly usesescapeHtml()on user content (line 112), which aligns with the best practices guidance and the security learnings for this codebase.
170-234: YAML renderer demonstrates safe interpolation pattern.The
yamlRendereruses a restricted dot-path interpolation (line 190) and consistently escapes all output (line 192), making it a good reference implementation for the documentation. This is the pattern you should highlight as the recommended alternative in the Markdown renderer section.
359-366: Best practices section reinforces security posture.The inclusion of "Escape output" (line 363) and "Fail gracefully" (line 361) as top practices is excellent and complements the security warnings earlier in the document.
libs/sdk/src/common/decorators/tool.decorator.ts (1)
192-226: Well-structured UI configuration with good documentation.The
ToolMetadataOptionstype properly integrates UI configuration with comprehensive JSDoc documentation and examples. The use ofUIContentSecurityPolicytype (line 218) correctly addresses the previous review feedback about avoiding inline type definitions.libs/sdk/src/tool/flows/call-tool.flow.ts (2)
2-25: New imports correctly wire Tool UI dependenciesThe added imports for
randomUUID,hasUIConfig, andScopeare all used infinalize()and align with the new Tool UI / platform-detection flow; no issues here.
359-455: Finalize flow + UI integration look correct and failure‑tolerantThe updated
finalize()logic (state destructuring, safeParseOutput, platform detection, UI rendering withrenderAndRegisterAsync,_metamerge, and final logging/response) is cohesive and keeps UI rendering strictly best‑effort—tool calls still succeed even if UI rendering fails. No blocking issues spotted in this block.apps/ui/mdx-demo/e2e/weather.e2e.ts (1)
10-21: LGTM! Excellent TypeScript typing.The imports and type definitions are clean and follow strict TypeScript guidelines. The
WeatherOutputinterface uses specific string literal types forunitsrather than generic strings, and noanytypes or non-null assertions are present.libs/testing/src/ui/ui-matchers.ts (2)
29-67: Helper utilities are type-safe and robust for mixed inputs
escapeRegex,extractUiHtml, andextractMetahandleunknowninputs cleanly, narrow types via runtime checks, and avoidanywhile still supporting both raw strings andToolResultWrapperobjects. This is a good fit for generic Jest matchers that may be used against different shapes.
77-96: Matcher semantics and widget metadata checks look consistent and well-factoredThe matchers correctly:
- Distinguish real rendered HTML from the
mdx-fallbackmarker.- Treat any of
ui/html,openai/outputTemplate, orui/mimeTypeas valid widget metadata (aligned with UIAssertions).- Expose a single
uiMatchersaggregate suitable forexpect.extend().The pass/fail messages are clear for both positive and
.notexpectations, which should make UI test failures easy to diagnose.Also applies to: 186-210, 305-314
libs/testing/src/ui/ui-assertions.ts (3)
39-71: Rendered-UI assertion covers key failure modes with clear diagnostics
assertRenderedUIvalidates_metapresence,ui/htmlexistence and type, and explicitly rejects themdx-fallbackmarker with a helpful error message pointing at MDX/React setup. This is strong coverage for the typical “no UI / fallback only” failure cases in tests.
106-144: XSS and structure assertions align well with UI security/quality goals
assertXssSafeandassertProperHtmlStructureimplement straightforward heuristics for script tags, inline handlers,javascript:URIs, escaped tags, and absence of any HTML tags. This gives you a solid baseline for validating that UI renderers both escape user content appropriately and actually produce HTML, matching the broader emphasis on XSS safety and proper rendering in your UI tests.
152-172: Regex usage in element/class assertions is safe given escapingAs in the matchers module, these assertions build regexes from
tag/className, butescapeRegexis applied and the rest of each pattern is simple. That keeps the patterns safe from injection and catastrophic backtracking, and they’re only used in test code. No change required; treating any static-analysis “regex from variable” warning here as a false positive is reasonable.libs/ui/src/base-template/default-base-template.ts (5)
40-44: JSDoc warning addresses previous security concern.The
@warningdocumentation clearly states thatheadContentis injected without escaping and must contain only trusted content. This addresses the concern raised in the previous review about the lack of sanitization for this parameter.
54-61: LGTM: Correct attribute escaping implementation.The
escapeAttrfunction properly escapes all HTML attribute metacharacters including quotes, angle brackets, and ampersands. The implementation is used correctly fortoolName,bodyClass, andcontainerClassattributes.
129-140: LGTM: Template head properly constructed with escaping.The HTML head section correctly:
- Escapes
toolNamein the title attribute (line 135)- Injects theme styles, polyfills, and bridge scripts from trusted internal functions
- Includes
headContentwith documented trust requirement
176-189: LGTM: Proper error handling and HTML escaping.The render function includes:
- Try-catch block for rendering errors (lines 176-188)
- Escaped error messages in
renderError(line 234)- Escaped JSON output in
defaultRenderer(line 210)- Correct
escapeHtmlimplementation (lines 250-257)All error paths properly sanitize output before display.
Also applies to: 232-236, 250-257
291-299: LGTM: Clean minimal template variant.The
createMinimalBaseTemplatefunction provides a clear convenience wrapper for lightweight widgets, properly delegating tocreateDefaultBaseTemplatewith framework features disabled.libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (5)
14-21: Scoped logger and per‑call tool logging improve observabilityUsing
scope.logger.child('call-tool-request-handler')and logging the resolvedtoolNameat Line [20] is a good pattern for correlating call‑tool traffic and filtering logs by handler, without changing behavior.
40-40: Includinge.outputin FlowControl warning log closes the diagnostics gapThe updated warn log at Line [40] now records
{ tool, type, output: e.output }, so abort/fail/handled/next cases preserve their diagnostic payloads in logs while still mapping to anInternalMcpErrorfor the client. This addresses the earlier observability concern around lost abort/fail reasons.
44-48: Structured error logging in the catch‑all path looks goodCapturing
tool: toolNameplus a structured{ name, message, stack }for realErrorinstances (Lines [45]-[47]) before delegating toformatMcpErrorResponse(e)provides clear server‑side diagnostics without changing the external error contract of this handler.
1-6: ImportingCallToolResultSchemafor response validation is correctThe import at line 5 is properly utilized in the handler's
safeParsevalidation at line 29, ensuring FlowControl.respond output conforms to the MCP protocol. The handler maintains strict typing withMcpHandler<CallToolRequest, CallToolResult>and uses Zod validation to preventunknownresponses from escaping to the protocol layer. Error handling logs validation failures with structured context, following SDK guidelines.
28-36: FlowControl.respond validation viaCallToolResultSchemais solid—no additional changes neededValidating
FlowControl.respondoutput withCallToolResultSchema.safeParse()(lines 29–31) before returning ensures only protocol-conformantCallToolResultobjects escape this handler. LoggingparseResult.error.issueson failure provides sufficient debugging detail while returning anInternalMcpErrorto the client. This is good defensive programming.The handler's generic type
McpHandler<CallToolRequest, CallToolResult>already captures the response type at the type system level. WhileresponseSchemais an optional property in theMcpHandlerinterface, it is not used by any handlers in the codebase and serves no functional purpose—the type system enforces correctness through the generic type parameters.libs/sdk/src/scope/flows/http.request.flow.ts (2)
245-347: SSE / HTTP handler wrappers are consistent; FlowControl filtering is correctThe new try/catch wrappers around
handleLegacySse,handleSse,handleStreamableHttp, andhandleStatelessHttpcorrectly treatFlowControlas expected control flow and only log real errors vialogError, then rethrow.The choice of
handled()for legacy/stateless handlers vsnext()forsse/streamable-httpalso seems intentional and consistent with a pattern where some transports fully consume the request while others can fall through to further middleware. As long as that matches existing expectations/tests, this section looks good to me.
413-457: logError helper is thorough and respects privacy; no issues
logErrordoes a good job of:
- Accepting
unknown, normalizing to name/message/stack.- Capturing additional
code/causewhen present.- Logging only selected, truncated request metadata (
user-agent,mcp-session-id,method,id) instead of the full payload.- Including a compact
statesnapshot.This is a solid centralization of error logging and should make debugging much easier without over‑logging sensitive data.
…and enhance component documentation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
libs/sdk/src/notification/notification.service.ts (1)
51-79: Previous feedback addressed: String matching now uses exact match.The implementation now correctly uses exact match (
===) for string patterns at line 67, aligning with thePlatformMappingEntrydocs that specify "string for exact match, RegExp for pattern". The past review concern has been resolved.apps/ui/mdx-demo/e2e/weather.e2e.ts (1)
12-20: Consider extracting the shared WeatherOutput interface.The
WeatherOutputinterface is duplicated identically in bothapps/ui/mdx-demo/e2e/weather.e2e.tsandapps/ui/react-demo/e2e/weather.e2e.ts. Extracting this to a shared location (e.g., a test utilities file or a shared types module) would reduce duplication and simplify maintenance if the output shape changes.docs/draft/docs/ui/api-reference/components.mdx (2)
1-751: Documentation diverges significantly from actual component schemas—critical accuracy issues require correction.This document contains multiple MAJOR discrepancies with the actual Zod schemas that would cause runtime validation failures for developers following it:
Button: Missing 'success' variant; missing properties (iconBefore, iconAfter, iconOnly, href, target, name, value, ariaLabel); ButtonGroup options incorrect (docs show 'align' which doesn't exist; missing 'attached').
Card: Variant 'outline' should be 'outlined'; missing 'filled' variant; missing properties (headerActions, clickable, href); CardGroup options completely wrong (docs show 'cols' and 'gap'—should be 'direction' and 'gap').
Modal: Property name mismatch ('closable' should be 'showClose'); missing properties (closeOnBackdrop, closeOnEscape, open, onClose).
Badge: Property 'rounded' should be 'pill'; variant 'xs' doesn't exist (sizes are sm/md/lg only); missing 'outline' variant; missing properties (icon, onRemove).
Avatar: Missing '2xl' size; missing 'rounded' shape option; missing 'none' status; missing properties (href, bgColor); alt marked optional but is required; missing align property for AvatarWithText.
Form: Properties use different names (helper vs hint, iconBefore/After vs prefix/suffix); missing properties (autocomplete, pattern, min, max, step); InputState includes 'warning' variant not documented; InputType includes 'hidden' type not documented.
Before publishing this as the authoritative reference, systematically verify and correct all documented options, property names, and variants against the actual schema files. The learnings indicate this should be derived from schemas—ensure that derivation is accurate and complete.
12-89: Update Button documentation to match ButtonOptionsSchema: add success variant, fix buttonGroup property, and complete ButtonOptions table.The documented Button variants (line 31) omit
'success', which exists in ButtonOptionsSchema. Update to:'primary' | 'secondary' | 'outline' | 'ghost' | 'danger' | 'success' | 'link'The ButtonOptions table (lines 27-40) is incomplete and missing 8 properties:
name,value,href,target,iconBefore,iconAfter,iconOnly,ariaLabel. Add these to the table with their types and descriptions.The buttonGroup options table (line 71) incorrectly lists
'align'as a property; the schema defines'attached'(boolean) instead. Replace the align row withattachedproperty.Add
successButtonpreset function to the Preset Buttons section (after line 87) to match the 'success' variant.
🧹 Nitpick comments (9)
apps/ui/mdx-demo/jest.config.ts (1)
1-35: Jest/SWC config looks correct; consider narrowing eslint disable and future-proofing extensionsThe Jest +
@swc/jestconfig itself looks fine and matches the usual ES2022/TypeScript + decorators setup; the preset path and coverage directory are reasonable for this app.Two small, optional refinements:
- Line 1: Instead of a blanket
/* eslint-disable */, consider either removing it (if not needed) or disabling only the specific rules that were problematic, so config drift is still caught by lint.- Lines 6–33: If this MDX demo is ever going to host React/JSX tests, you may want to extend both the transform pattern and
moduleFileExtensionsto includetsx/jsxand adjust SWC parser options accordingly; otherwise the current scope is fine for pure.ts/.js.If you’d like stronger typing, you could also (optionally) import Jest’s
Configtype and annotate the export to catch typos in keys at compile time.libs/sdk/src/notification/notification.service.ts (1)
87-93: Consider tightening thegptkeyword match to reduce false positive risk.The keyword
gpton line 91 is fairly short and could theoretically match unintended client names. While unlikely in practice, you may want to require a word boundary or hyphen, similar to how Gemini detection uses more specific patterns.- if (lowerIdentifier.includes('chatgpt') || lowerIdentifier.includes('openai') || lowerIdentifier.includes('gpt')) { + // Match 'gpt' only as a word boundary to avoid false positives + if (lowerIdentifier.includes('chatgpt') || lowerIdentifier.includes('openai') || /\bgpt\b/.test(lowerIdentifier)) { return 'openai'; }apps/ui/react-demo/e2e/weather.e2e.ts (2)
12-20: Consider extracting the shared WeatherOutput interface.The
WeatherOutputinterface is duplicated identically in bothapps/ui/react-demo/e2e/weather.e2e.tsandapps/ui/mdx-demo/e2e/weather.e2e.ts. Extracting this to a shared location (e.g., a test utilities file or a shared types module) would reduce duplication and simplify maintenance if the output shape changes.
31-135: Expand test coverage with cross-platform and validation tests.The test suite has excellent coverage for rendering, XSS safety, and data binding. However, consider adding:
Cross-platform testing: Test behavior across different platform configurations (OpenAI, Claude, Gemini, ngrok) to ensure consistent rendering and data binding.
Invalid input validation: Add tests for edge cases such as:
- Invalid or unknown location values
- Invalid units (neither 'celsius' nor 'fahrenheit')
- Missing required parameters
- Boundary conditions (e.g., extreme temperature values)
Based on learnings, these tests help ensure comprehensive validation and maintain 95%+ coverage standards.
apps/ui/mdx-demo/e2e/weather.e2e.ts (1)
32-186: Expand test coverage with cross-platform and validation tests.The MDX test suite provides excellent coverage for template rendering, custom components, Markdown elements, and XSS safety. However, consider adding:
Cross-platform testing: Test behavior across different platform configurations (OpenAI, Claude, Gemini, ngrok) to ensure MDX templates render consistently across platforms.
Invalid input validation: Add tests for edge cases such as:
- Invalid or unknown location values
- Malformed input that might break MDX rendering
- Missing required parameters
- Boundary conditions
Based on learnings, these tests help ensure comprehensive validation and maintain 95%+ coverage standards.
docs/draft/docs/ui/api-reference/components.mdx (2)
525-556: Fix repetitive sentence structure in list component descriptions.Lines 526 and 543 both begin with "Creates a..." which creates unnecessarily repetitive phrasing. Consider varying the descriptions to improve readability.
Suggested changes:
-### `permissionList(items)` - -Creates a permission checklist. +### `permissionList(items)` + +Displays a permission checklist.-### `featureList(items)` - -Creates a feature comparison list. +### `featureList(items)` + +Displays a feature comparison list.
722-751: Consider documenting strict schema mode for unknown property rejection.The Zod Schemas section demonstrates
safeParse()usage well. However, per the codebase learnings, all component schemas should use.strict()mode to reject unknown properties. Consider adding documentation about this validation guarantee:All components export Zod schemas for runtime validation. +**Strict Validation**: All component schemas use Zod's `.strict()` mode to reject unknown properties, ensuring predictable behavior and preventing accidental configuration errors. \`\`\`typescriptThis clarifies to users that the schemas provide strict property validation as a safety guarantee.
libs/sdk/src/common/decorators/tool.decorator.ts (2)
185-189: Avoidanyin React-style template signatureThe React-style variant
(props: any) => anyplus the eslint suppression undermines strict typing for UI templates. To stay aligned with the project’s “noany” guideline, consider switching tounknown(or a small alias) instead, e.g.:type ReactComponentLike = (props: unknown) => unknown; type __UITemplateType = | ((ctx: { input: unknown; output: unknown; structuredContent?: unknown; helpers: TemplateHelpers }) => string) | string | ReactComponentLike;This preserves flexibility without leaking
anyinto consumers.
191-225: Reuse a shared UI config type instead of re-declaring theuishape
ToolMetadataOptionsbuilds onToolMetadataand then redefinesuiinline. IfToolMetadata(or a nearby module) already exposes a canonicalToolUIConfig<In, Out>type, intersecting with a second inlineuidefinition risks divergence if fields change in one place but not the other.Consider basing
ToolMetadataOptionson something like:
type __ToolMetadataBase = Omit<ToolMetadata<...>, 'ui'>;- then
ui?: /* shared ToolUIConfig type */;so there is a single source of truth for the UI config shape.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/ui/mdx-demo/e2e/weather.e2e.ts(1 hunks)apps/ui/mdx-demo/jest.config.ts(1 hunks)apps/ui/react-demo/e2e/weather.e2e.ts(1 hunks)apps/ui/react-demo/jest.config.ts(1 hunks)docs/draft/docs/ui/api-reference/components.mdx(1 hunks)libs/sdk/src/common/decorators/tool.decorator.ts(2 hunks)libs/sdk/src/common/types/options/logging.options.ts(1 hunks)libs/sdk/src/notification/notification.service.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/ui/react-demo/jest.config.ts
- libs/sdk/src/common/types/options/logging.options.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/ui/mdx-demo/e2e/weather.e2e.tsapps/ui/mdx-demo/jest.config.tslibs/sdk/src/common/decorators/tool.decorator.tsapps/ui/react-demo/e2e/weather.e2e.tslibs/sdk/src/notification/notification.service.ts
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/ui/api-reference/components.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/ui/api-reference/components.mdx
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/common/decorators/tool.decorator.tslibs/sdk/src/notification/notification.service.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/common/decorators/tool.decorator.tslibs/sdk/src/notification/notification.service.ts
🧠 Learnings (18)
📚 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/ui/mdx-demo/e2e/weather.e2e.tsapps/ui/mdx-demo/jest.config.tsapps/ui/react-demo/e2e/weather.e2e.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/ui/mdx-demo/e2e/weather.e2e.tsdocs/draft/docs/ui/api-reference/components.mdxapps/ui/react-demo/e2e/weather.e2e.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/ui/mdx-demo/e2e/weather.e2e.tsapps/ui/react-demo/e2e/weather.e2e.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 : Follow the preset pattern for hierarchical configurations across the codebase
Applied to files:
apps/ui/mdx-demo/jest.config.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 : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
docs/draft/docs/ui/api-reference/components.mdx
📚 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:
docs/draft/docs/ui/api-reference/components.mdx
📚 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:
docs/draft/docs/ui/api-reference/components.mdxlibs/sdk/src/common/decorators/tool.decorator.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:
docs/draft/docs/ui/api-reference/components.mdx
📚 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:
docs/draft/docs/ui/api-reference/components.mdx
📚 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:
docs/draft/docs/ui/api-reference/components.mdx
📚 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/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/ui/react-demo/e2e/weather.e2e.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/notification/notification.service.ts
🧬 Code graph analysis (2)
apps/ui/mdx-demo/e2e/weather.e2e.ts (3)
libs/testing/src/index.ts (3)
test(145-145)expect(162-162)UIAssertions(211-211)libs/testing/src/ui/ui-assertions.ts (1)
UIAssertions(38-242)libs/testing/src/ui/index.ts (1)
UIAssertions(18-18)
libs/sdk/src/notification/notification.service.ts (4)
libs/sdk/src/notification/index.ts (4)
ClientInfo(11-11)AIPlatformType(12-12)detectPlatformFromUserAgent(14-14)detectAIPlatform(13-13)libs/sdk/src/common/types/options/session.options.ts (2)
PlatformMappingEntry(11-16)PlatformDetectionConfig(21-33)libs/sdk/src/common/types/auth/session.types.ts (1)
AIPlatformType(13-21)libs/sdk/src/common/interfaces/tool.interface.ts (1)
clientInfo(149-155)
🪛 LanguageTool
docs/draft/docs/ui/api-reference/components.mdx
[style] ~526-~526: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., ]); ### `permissionList(items)` Creates a permission checklist. typescript ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~543-~543: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e }, ]); ### `featureList(items)` Creates a feature comparison list. typescri...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (17)
libs/sdk/src/notification/notification.service.ts (7)
7-8: LGTM!The type imports and re-exports are well-structured. Re-exporting
AIPlatformTypemaintains backwards compatibility for consumers who import from this module.Also applies to: 16-19
40-49: LGTM!The
ClientInfointerface is well-defined and aligns with MCP protocol conventions. Based on learnings, this TypeScript-first schema validation approach matches the framework philosophy.
133-159: LGTM!The function handles all cases correctly: undefined input, custom mappings, customOnly flag, and default detection fallback. The control flow is clear and the return type is properly constrained.
161-187: LGTM!The function is well-implemented with proper null/undefined handling. The flow matches
detectPlatformFromUserAgentwhich provides consistency across both detection entry points.
226-229: LGTM!The new optional fields appropriately extend
RegisteredServerto support platform detection. Making them optional allows for the two-phase initialization (register server first, set client info on initialize).
707-733: LGTM!The
setClientInfomethod follows the established patterns in this service: validate session existence, update the registered server state, and log the outcome. The auto-detection using scope configuration provides good extensibility for custom platform mappings.
735-755: LGTM!Both getter methods are well-designed. Notably,
getPlatformTypereturnsAIPlatformType(not| undefined) by defaulting to'unknown', which provides a cleaner API for callers.docs/draft/docs/ui/api-reference/components.mdx (9)
1-11: ✓ Well-structured documentation introduction.The frontmatter and description accurately frame this as the canonical API reference. This sets the right expectation that content here should be derived from and match the component Zod schemas.
92-136: ✓ Card section is well-documented.The options, examples, and structure are clear and comprehensive. No conflicts identified with prior review feedback.
139-198: ✓ Badge section is clear and complete.Good variety of preset badges and status dots documented with examples. Variant list is comprehensive.
201-261: ✓ Alert section appears aligned with component documentation.The Alert variant list (including
'neutral') and option properties (showIcon: boolean,icon: string) appear to address prior review concerns. The toast and toastContainer sections are well-documented.
268-414: ✓ Form Components section is comprehensive and well-structured.All major form inputs are documented with clear examples. Form layout helpers are especially useful. Options are detailed and well-described.
418-481: ✓ Table and pagination sections are well-documented.The render function pattern for custom cell rendering is useful, and the pagination integration example is clear. Options are comprehensive.
560-618: ✓ Avatar section is well-documented with useful composition patterns.The examples effectively demonstrate both image and fallback (initials) scenarios. AvatarGroup and AvatarWithText provide flexible composition options.
688-718: ✓ HTMX Options section is comprehensive and serves as definitive reference.The HtmxOptions table is appropriately detailed for a central API reference. The example demonstrates practical usage clearly.
621-685: Verify Modal, Drawer, and related function signatures match the actual component exports and schemas.A prior review indicated potential divergence between this documentation (which shows
modal(content, options),drawer(content, options), andconfirmModal(options)) and the actual component implementations. Verify that:
- Function signatures (
modal(),drawer(),confirmModal(),modalTrigger()) exactly match the exports inlibs/ui/src/components/modal/modal.ts- Option properties in
ModalOptions,DrawerOptions, andConfirmModalOptionstables match the corresponding Zod schemas (must use.strict()mode per coding standards)- All size/position enum values are consistent with schema definitions
Check against
libs/ui/src/components/modal/modal.schema.tsand implementation files.libs/sdk/src/common/decorators/tool.decorator.ts (1)
12-15: UI-related imports are consistent and DRY-friendlyImporting
UIContentSecurityPolicyandTemplateHelpersfrom the shared metadata modules keeps UI typing centralized and avoids ad-hoc inline shapes. No issues here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/sdk/src/common/decorators/tool.decorator.ts (1)
177-187: Document whyanyis used in the React template branchThe
__UITemplateTypefunction branch usesunknownfor the structured context, which is good, but the React-style signature falls back toany:// eslint-disable-next-line @typescript-eslint/no-explicit-any | ((props: any) => any); // React component signatureGiven you want to accept arbitrary React/MDX components without importing
reacttypes,anyhere is a pragmatic choice to keep those components assignable. I’d suggest making that intent explicit in the comment so future refactors don’t “fix” it tounknownand break assignment:- // eslint-disable-next-line @typescript-eslint/no-explicit-any - | ((props: any) => any); // React component signature + // eslint-disable-next-line @typescript-eslint/no-explicit-any + | ((props: any) => any); // React/MDX component; `any` is intentional to keep arbitrary components assignable without pulling React types
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/sdk/src/common/decorators/tool.decorator.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/common/decorators/tool.decorator.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/common/decorators/tool.decorator.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/common/decorators/tool.decorator.ts
🧠 Learnings (7)
📚 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/common/decorators/tool.decorator.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/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.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/common/decorators/tool.decorator.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/sdk/src/common/decorators/tool.decorator.ts
🧬 Code graph analysis (1)
libs/sdk/src/common/decorators/tool.decorator.ts (4)
libs/sdk/src/common/metadata/tool.metadata.ts (1)
ToolMetadata(163-220)libs/sdk/src/common/metadata/tool-ui.metadata.ts (2)
TemplateHelpers(40-70)UIContentSecurityPolicy(17-31)libs/ui/src/runtime/index.ts (2)
TemplateHelpers(25-25)UIContentSecurityPolicy(23-23)libs/ui/src/runtime/types.ts (2)
TemplateHelpers(40-55)UIContentSecurityPolicy(19-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
libs/sdk/src/common/decorators/tool.decorator.ts (2)
12-15: Shared UI types import is consistent and DRYPulling
UIContentSecurityPolicyandTemplateHelpersfrom the shared metadata surface keeps the decorator aligned with the central UI typing model and avoids inline CSP/interface duplication. This matches the hierarchical config pattern used elsewhere in the SDK.Based on learnings, this follows the guidance to centralize common config/record types in shared metadata surfaces.
169-176: ToolMetadataOptions + UI surface wiring looks soundComposing
ToolMetadataOptions<I, O>from__ToolMetadataBase<I, O>plus auiblock keeps the decorator overloads tied intoToolInputOf/ToolOutputOfwhile exposing a focused UI surface:
template: __UITemplateTypelines up with the new template union and TemplateHelpers.csp?: UIContentSecurityPolicycorrectly reuses the shared CSP type instead of redefining the shape.widgetAccessible,displayMode, andwidgetDescriptionread clearly and don’t introduce behavioral changes to the existing execution surface.This structure should work cleanly with the existing
Tooloverloads that now parameterize onToolMetadataOptions<I, O>.Also applies to: 188-222
Summary by CodeRabbit
New Features
Enhancements
Tests / Docs
✏️ Tip: You can customize this high-level summary in your review settings.