-
Notifications
You must be signed in to change notification settings - Fork 169
Add mcp apps ux to builder #1038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds protocol awareness (openai-apps vs mcp-apps) and timezone support in the playground. Introduces MCP Apps CSP handling end-to-end: server accepts/returns Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
server/routes/mcp/apps.ts (1)
14-17: Type definition mirrors client-side; prudent alignment.The
CspModetype is declared locally rather than shared. Consider extracting to a shared types module if this type proliferates across server and client boundaries.client/src/components/ui-playground/UIPlaygroundTab.tsx (1)
200-238: Protocol detection logic is methodical and handles edge cases.The cascade—specific tool first, then server-wide inference, defaulting to
nullfor mixed—ensures predictable UI behavior. The comment at line 235 clarifies the mixed-protocol fallback.Consider extracting this detection logic to a custom hook (e.g.,
useProtocolDetection) for improved testability and reuse.client/src/components/ui-playground/PlaygroundMain.tsx (2)
469-538: Device toggle groups are duplicated across protocol branches.The ToggleGroup for device selection (lines 472-502 and 506-536) is nearly identical between ChatGPT and MCP Apps controls. This duplication invites maintenance drift.
Extract to a shared component:
function DeviceToggleGroup({ deviceType, onDeviceTypeChange }: { deviceType: DeviceType; onDeviceTypeChange?: (type: DeviceType) => void; }) { return ( <ToggleGroup type="single" value={deviceType} onValueChange={(v) => v && onDeviceTypeChange?.(v as DeviceType)} className="gap-0.5" > {/* ... toggle items ... */} </ToggleGroup> ); }
541-803: Substantial duplication in protocol-specific control sections.The center section duplicates locale selectors, CSP selectors, capability toggles, and SafeAreaEditor between
showChatGPTControlsandshowMCPAppsControlsbranches. The only material differences are:
- CSP state binding (
cspModevsmcpAppsCspMode)- Tooltip text for MCP Apps CSP selector
This ~250 lines of near-identical JSX could be consolidated into a single parameterized block or extracted components.
const activeCspMode = showMCPAppsControls ? mcpAppsCspMode : cspMode; const setActiveCspMode = showMCPAppsControls ? setMcpAppsCspMode : setCspMode; // Then use activeCspMode/setActiveCspMode in a single control block
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/src/components/chat-v2/mcp-apps-renderer.tsx(5 hunks)client/src/components/ui-playground/PlaygroundMain.tsx(3 hunks)client/src/components/ui-playground/UIPlaygroundTab.tsx(3 hunks)client/src/stores/ui-playground-store.ts(8 hunks)server/routes/mcp/apps.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
client/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
client/**/components/**/*.{ts,tsx}: Use functional React components with React.FC typing
Implement strict TypeScript types for React component props with Props interface definitions
Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Use Radix UI components for Dialog, Dropdown menus, Form controls, and Tooltips
Design responsive layouts using Tailwind breakpoint system, grid layouts, flex containers, and container queries
Ensure accessibility compliance with ARIA attributes, keyboard navigation, focus management, and screen reader support
Files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/ui-playground/UIPlaygroundTab.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
client/**/{components,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/ui-playground/UIPlaygroundTab.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
client/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use Zustand for global state management with proper store creation, action definitions, selector optimization, and middleware usage
Files:
client/src/stores/ui-playground-store.ts
server/**/*.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Implement TypeScript for type safety throughout the codebase
Files:
server/routes/mcp/apps.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/servers/**/*.{ts,tsx} : Synchronize MCP connection state, request tracking, response handling, and error management with global state
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Implement MCP server management UI with connection list, status indicators, quick actions, and group management
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Implement MCP server management UI with connection list, status indicators, quick actions, and group management
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/servers/**/*.{ts,tsx} : Synchronize MCP connection state, request tracking, response handling, and error management with global state
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/stores/ui-playground-store.tsclient/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Provide transport protocol selection UI with protocol options, configuration forms, validation rules, and default presets
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/ui-playground/UIPlaygroundTab.tsxclient/src/stores/ui-playground-store.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{stores/chat,hooks/llm}/**/*.{ts,tsx} : Manage context with window size tracking, token counting, context pruning, and state persistence for chat sessions
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/stores/ui-playground-store.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/types/mcp.ts : Define MCP protocol types in TypeScript type definition files
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/stores/ui-playground-store.tsserver/routes/mcp/apps.tsclient/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/chat/**/*.{ts,tsx} : Implement AI model state handling including model selection state, generation parameters, stream management, and history persistence
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/stores/ui-playground-store.ts
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/{app,index,routes/**}.ts : Use Hono.js for API routing and middleware in the backend
Applied to files:
server/routes/mcp/apps.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{components,hooks}/**/*.{ts,tsx} : Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Applied to files:
client/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/**/*.{ts,tsx} : Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Applied to files:
client/src/components/chat-v2/mcp-apps-renderer.tsx
🧬 Code graph analysis (1)
client/src/components/ui-playground/PlaygroundMain.tsx (1)
client/src/stores/ui-playground-store.ts (2)
DeviceType(13-13)CspMode(15-15)
⏰ 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: Cursor Bugbot
🔇 Additional comments (11)
server/routes/mcp/apps.ts (2)
164-173: CSP computation logic is sound.The branching is clear: permissive mode provides broad HTTPS allowances while widget-declared defers to metadata (with SandboxedIframe handling restrictive defaults). The comment accurately describes the behavior.
177-182: Response structure appropriately exposes CSP metadata to clients.Returning both the computed CSP and the effective mode enables the client to render and log accordingly. Cache-control headers prevent stale CSP configurations.
client/src/stores/ui-playground-store.ts (5)
16-16: AppProtocol type includesnullfor indeterminate state—sensible design.The tri-state (
"openai-apps" | "mcp-apps" | null) accommodates mixed-protocol servers or no selection gracefully.
55-62: TimeZone addition follows IANA standard per SEP-1865.The field complements locale for accurate temporal rendering within widgets.
154-161: Timezone initialization with graceful fallback.Using
Intl.DateTimeFormat().resolvedOptions().timeZonewith a"UTC"fallback ensures robustness across browsers with varying Intl support.
217-227: Protocol reset on tool change prevents stale state.Clearing
selectedProtocolwhen selecting a new tool ensures UIPlaygroundTab re-derives the protocol from fresh metadata—a clean state machine transition.
314-316: Actions follow Zustand conventions.Simple, direct state mutations via
set(). No concerns.client/src/components/chat-v2/mcp-apps-renderer.tsx (2)
84-88: CSP mode derivation is contextually aware.When playground is active, the store-driven mode governs security policy; otherwise, permissive mode applies. This distinction appropriately separates development-time testing from production defaults.
110-173: Effect re-fetches on cspMode change—intentional but worth noting.Changing CSP mode will re-store and re-fetch the widget. This is correct behavior for applying new security constraints, though users should understand mode changes trigger widget reloads.
Confirm this reload behavior is acceptable UX. If rapid toggling causes disruption, consider debouncing or confirming with users before re-fetching.
client/src/components/ui-playground/PlaygroundMain.tsx (2)
281-285: Protocol-driven UI branching is cleanly expressed.The boolean flags
showChatGPTControlsandshowMCPAppsControlsprovide readable conditional rendering. The default to ChatGPT controls whenselectedProtocol === nullmaintains backward compatibility.
805-824: Theme toggle correctly shared across both protocols.Placing it outside the conditional branches ensures consistent access regardless of selected protocol. Well structured.
| // Read CSP mode from query param (allows override for testing) | ||
| const cspModeParam = c.req.query("csp_mode") as CspMode | undefined; | ||
|
|
||
| const { serverId, resourceUri, cspMode: storedCspMode } = widgetData; | ||
|
|
||
| // Use query param override if provided, otherwise use stored mode | ||
| const effectiveCspMode = cspModeParam ?? storedCspMode ?? "permissive"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Query parameter lacks validation against allowed values.
The csp_mode query parameter is cast directly to CspMode without verifying it's a valid value. An attacker or misconfigured client could pass an arbitrary string, which would flow through as effectiveCspMode.
- // Read CSP mode from query param (allows override for testing)
- const cspModeParam = c.req.query("csp_mode") as CspMode | undefined;
+ // Read CSP mode from query param (allows override for testing)
+ const rawCspMode = c.req.query("csp_mode");
+ const cspModeParam: CspMode | undefined =
+ rawCspMode === "permissive" || rawCspMode === "widget-declared"
+ ? rawCspMode
+ : undefined;🤖 Prompt for AI Agents
In server/routes/mcp/apps.ts around lines 117 to 123, the csp_mode query param
is cast directly to CspMode without validation; update the code to validate that
cspModeParam is one of the allowed CspMode values (e.g., check against the
CspMode enum or a constant array of allowed strings) before using it, and if
it’s invalid either ignore it (fall back to storedCspMode or the "permissive"
default) and optionally log a warning or return a 400 error; ensure the final
effectiveCspMode assignment uses only validated values so arbitrary strings
cannot flow through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/components/chat-v2/mcp-apps-renderer.tsx (1)
207-242: AddtimeZonetoWidgetGlobalsto fix TypeScript type errorsThe code at lines 207–242 passes
timeZoneto bothsetWidgetDebugInfoandsetWidgetGlobals, but theWidgetGlobalsinterface inclient/src/stores/widget-debug-store.tsdoesn't declare this field. This causes TypeScript excess-property errors.Extend
WidgetGlobalsto includetimeZone:export interface WidgetGlobals { theme: "light" | "dark"; displayMode: "inline" | "pip" | "fullscreen"; maxHeight?: number; locale?: string; + timeZone?: string; safeArea?: { insets: { top: number; bottom: number; left: number; right: number }; }; userAgent?: { device: { type: string }; capabilities: { hover: boolean; touch: boolean }; }; }
♻️ Duplicate comments (1)
server/routes/mcp/apps.ts (1)
49-79: Validate and normalize CSP mode from both body and query instead of using type assertions.Right now,
cspModefrom the JSON body andcsp_modefrom the query string are treated asCspModevia assertion, so any arbitrary string can flow intoWidgetData.cspMode,effectiveCspMode, and the responsecspMode. That breaks the union guarantee and can confuse clients expecting only"permissive"or"widget-declared". This also overlaps with the earlier review note aboutcsp_mode; adding validation for the body closes the remaining gap.You can normalize both sources to the allowed values and fall back to
"permissive"/undefinedfor anything else:apps.post("/widget/store", async (c) => { try { const body = await c.req.json(); const { serverId, resourceUri, toolInput, toolOutput, toolId, toolName, theme, protocol, - cspMode, + cspMode: rawCspMode, } = body; + + const cspMode: CspMode = + rawCspMode === "permissive" || rawCspMode === "widget-declared" + ? rawCspMode + : "permissive"; @@ widgetDataStore.set(toolId, { @@ toolName, theme: theme ?? "dark", protocol: protocol ?? "mcp-apps", - cspMode: cspMode ?? "permissive", // Default to permissive mode + cspMode, // already normalized with "permissive" default timestamp: Date.now(), }); @@ - // Read CSP mode from query param (allows override for testing) - const cspModeParam = c.req.query("csp_mode") as CspMode | undefined; + // Read CSP mode from query param (allows override for testing) + const rawCspModeParam = c.req.query("csp_mode"); + const cspModeParam: CspMode | undefined = + rawCspModeParam === "permissive" || rawCspModeParam === "widget-declared" + ? rawCspModeParam + : undefined; @@ - // Use query param override if provided, otherwise use stored mode - const effectiveCspMode = cspModeParam ?? storedCspMode ?? "permissive"; + // Use query param override if provided, otherwise use stored mode + const effectiveCspMode = cspModeParam ?? storedCspMode ?? "permissive";This keeps runtime behavior predictable, ensures only valid CSP modes reach downstream consumers, and still preserves the intended override semantics.
Also applies to: 117-124
🧹 Nitpick comments (3)
server/routes/mcp/sandbox-proxy.html (1)
127-146: CSP violation listener broadcasts with wildcard origin—validate at host level.The violation listener posts to any origin via
postMessage(violation, '*'). While the double-iframe architecture and opaque srcdoc context mitigate risk, tightening the second argument to the actual host origin would improve security posture and prevent accidental information leakage to unintended observers.Consider storing the host origin on sandbox-resource-ready and using it in postMessage:
+ let hostOrigin = '*'; // fallback, but should be set from parent window.addEventListener("message", async (event) => { if (event.source === window.parent) { if (event.data && event.data.method === "ui/notifications/sandbox-resource-ready") { + hostOrigin = event.origin; // Capture host origin const { html, sandbox, csp, permissive } = event.data.params || {};Then update the violation listener:
- window.parent.postMessage(violation, '*'); + window.parent.postMessage(violation, hostOrigin);server/routes/mcp/apps.ts (1)
14-29: CspMode and WidgetData CSP field look good; consider centralizing the type.The local
CspModealias andWidgetData.cspModefield are clear and align with the client-side union, but this duplicates the CSP mode definition across client and server. Consider defining the MCP-related CSP types in a shared server types module (e.g., underserver/types/mcp.ts) and importing them here to avoid drift between implementations. Based on learnings, this matches the guidance to centralize MCP protocol types.client/src/components/ui-playground/PlaygroundMain.tsx (1)
265-289: Protocol‑aware CSP mode selection is correct; consider deduping header controlsThe logic that derives
activeCspMode/setActiveCspModefromselectedProtocoland gates UI viashowChatGPTControls/showMCPAppsControlsaligns with the intended split between ChatGPT Apps and MCP Apps; CSP changes will hit the right store field and thus the right renderer. The two protocol branches in the header are nearly identical aside from which CSP setter they use, so if this area evolves further it may be worth extracting a small shared “device/locale/capabilities/safe‑area” control group with a protocol‑specific CSP selector hook.Also applies to: 485-829
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/src/components/chat-v2/mcp-apps-renderer.tsx(17 hunks)client/src/components/chat-v2/thread.tsx(11 hunks)client/src/components/ui-playground/PlaygroundMain.tsx(3 hunks)client/src/components/ui/sandboxed-iframe.tsx(4 hunks)client/src/stores/widget-debug-store.ts(3 hunks)server/routes/mcp/apps.ts(6 hunks)server/routes/mcp/sandbox-proxy.html(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
client/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use Zustand for global state management with proper store creation, action definitions, selector optimization, and middleware usage
Files:
client/src/stores/widget-debug-store.ts
server/**/*.ts
📄 CodeRabbit inference engine (server/CLAUDE.md)
Implement TypeScript for type safety throughout the codebase
Files:
server/routes/mcp/apps.ts
client/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
client/**/components/**/*.{ts,tsx}: Use functional React components with React.FC typing
Implement strict TypeScript types for React component props with Props interface definitions
Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Use Radix UI components for Dialog, Dropdown menus, Form controls, and Tooltips
Design responsive layouts using Tailwind breakpoint system, grid layouts, flex containers, and container queries
Ensure accessibility compliance with ARIA attributes, keyboard navigation, focus management, and screen reader support
Files:
client/src/components/ui/sandboxed-iframe.tsxclient/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/chat-v2/thread.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
client/**/{components,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Files:
client/src/components/ui/sandboxed-iframe.tsxclient/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/chat-v2/thread.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
🧠 Learnings (12)
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/types/mcp.ts : Define MCP protocol types in TypeScript type definition files
Applied to files:
server/routes/mcp/apps.tsclient/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/{app,index,routes/**}.ts : Use Hono.js for API routing and middleware in the backend
Applied to files:
server/routes/mcp/apps.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Provide transport protocol selection UI with protocol options, configuration forms, validation rules, and default presets
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{stores/chat,hooks/llm}/**/*.{ts,tsx} : Manage context with window size tracking, token counting, context pruning, and state persistence for chat sessions
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsxclient/src/components/chat-v2/thread.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/chat/**/*.{ts,tsx} : Implement AI model state handling including model selection state, generation parameters, stream management, and history persistence
Applied to files:
client/src/components/ui-playground/PlaygroundMain.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Implement message threading with thread components, collapse/expand functionality, navigation, and search capabilities
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Implement code highlighting with syntax detection, theme support, copy functionality, and line numbers in code components
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Build chat interface with message components, input handling, stream rendering, and history management
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/**/*.{ts,tsx} : Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Applied to files:
client/src/components/chat-v2/thread.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/**/*.{ts,tsx} : Use Radix UI components for Dialog, Dropdown menus, Form controls, and Tooltips
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/servers/**/*.{ts,tsx} : Synchronize MCP connection state, request tracking, response handling, and error management with global state
Applied to files:
client/src/components/chat-v2/mcp-apps-renderer.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{components,hooks}/**/*.{ts,tsx} : Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Applied to files:
client/src/components/chat-v2/mcp-apps-renderer.tsx
🧬 Code graph analysis (2)
server/routes/mcp/apps.ts (1)
client/src/stores/ui-playground-store.ts (1)
CspMode(15-15)
client/src/components/ui-playground/PlaygroundMain.tsx (2)
client/src/stores/ui-playground-store.ts (3)
useUIPlaygroundStore(212-346)DeviceType(13-13)CspMode(15-15)client/src/components/ui-playground/SafeAreaEditor.tsx (1)
SafeAreaEditor(146-237)
🔍 Remote MCP
Additional Context for PR Review
Based on the gathered information, here's relevant context for effectively reviewing this MCP Apps UX and CSP implementation PR:
MCP Apps Background
The PR implements SEP-1865 (MCP Apps Extension proposal), which standardizes support for interactive user interfaces in the Model Context Protocol, building on proven work from MCP-UI and OpenAI Apps SDK. The specification supports only text/html content, rendered in sandboxed iframes.
Security is addressed through multiple layers: Iframe sandboxing (all UI content runs in sandboxed iframes with restricted permissions), predeclared templates (hosts can review HTML content before rendering), and auditable messages (all UI-to-host communication goes through loggable JSON-RPC).
CSP Mode Architecture in This PR
The PR implements two distinct CSP handling modes:
-
"permissive" mode: Follows the principle of least privilege by granting content only the minimum level of capability necessary to do its job.
-
"widget-declared" mode: Uses CSP metadata from the widget itself to enforce security restrictions.
CSP Violation Handling Best Practices
Content Security Policy (CSP) is a feature that helps to prevent or minimize the risk of certain types of security threats by instructing the browser to place restrictions on what the code is allowed to do; the primary use case is controlling which resources, particularly JavaScript resources, a document is allowed to load.
CSP violation reporting is handled via reporting directives that control the destination URL for CSP violation reports, which aligns with the PR's CSP violation logging and debug store recording.
Key Review Considerations
-
State Management: The PR adds protocol-aware CSP mode selection (
mcpAppsCspModefor MCP Apps vs.cspModefor ChatGPT Apps), with proper state reset on tool changes. -
Client-Server Consistency: CSP mode is propagated from client to server via query parameters (
?csp_mode=) and stored in widget data, with server-side effective CSP mode computation. -
Sandbox Iframe Integration: The
permissiveflag is passed to sandboxed iframe props, enabling the guest UI to understand CSP enforcement policy. -
Violation Lifecycle: CSP violations are cleared when CSP mode changes, preventing stale violation data from appearing in debug views.
[::web_search::], [::web_search::], [::web_search::]
⏰ 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: Cursor Bugbot
🔇 Additional comments (9)
server/routes/mcp/sandbox-proxy.html (4)
154-180: HTML injection is robust and correctly combines CSP meta and violation listener.The refactored
injectCSPnow properly injects both the CSP meta tag and the violation listener script, with fallbacks for various HTML structure variants. The combined injection approach is cleaner than the prior single-injection behavior.
208-231: Permissive CSP mode correctly branches and applies intentionally broad policy.The conditional logic properly detects the
permissiveflag and applies the appropriate CSP injection path. The permissive policy is deliberately broad (matching the design principle of maximum capability when CSP restrictions are relaxed), and both paths correctly invokeinjectCSPand update the inner iframe.
80-80: Media-src directive additions are consistent and appropriate.Adding
media-srcto both the default and declared CSP paths properly extends CSP coverage to media resources (audio/video), ensuring widgets can embed multimedia content. The directive is applied consistently across all CSP modes (restrictive, declared, and permissive).Also applies to: 114-114
7-8: Proxy's own CSP is intentionally permissive to avoid constraining guest content.The CSP at the proxy level deliberately permits broad resources (unsafe-eval, development CDNs, localhost) to ensure that restrictions don't leak to nested guest iframes. This aligns with the architecture goal stated in the comment.
Verify that this proxy document's CSP is appropriate for its deployment context (development-only or production use). If deployed to production, consider tightening the CSP to remove development CDNs and localhost references, and evaluate whether
unsafe-evalis necessary.server/routes/mcp/apps.ts (1)
154-175: CSP logging and response flags align well with the two-mode CSP model.The consolidated log line with
effectiveCspModeplus widget-declared CSP details, and the response shape{ html, csp: …, permissive, cspMode, prefersBorder }, cleanly expresses the CSP behavior to downstream components. SkippingcspwhenisPermissiveistruewhile still returning the explicitpermissiveflag gives the sandbox layer enough information to decide whether to inject CSP. This looks consistent with the intended SEP‑1865 semantics.client/src/stores/widget-debug-store.ts (1)
106-108: CSP violation reset logic is consistent and side‑effect free
clearCspViolationscleanly no‑ops when there’s no CSP state, preserves existing CSP metadata, and only clears the violations array while updatingupdatedAt. This matches how MCP Apps re-fetch on CSP mode changes and should keep the debug panel accurate without losing configuration context.Also applies to: 227-243
client/src/components/ui/sandboxed-iframe.tsx (1)
43-45: CSP “permissive” plumbing and violation forwarding look correctThreading
permissivethrough to the sandbox and treatingmcp-apps:csp-violationas a first‑class side channel (with origin/source checks still enforced) is a clean separation from the JSON‑RPC flow, and the effect dependencies are updated appropriately.Also applies to: 73-74, 132-136, 170-175
client/src/components/chat-v2/thread.tsx (1)
281-301: UI‑type‑aware debug tabs are well factoredPropagating
uiTypeintoToolPartand usinguseMemoto derivedebugOptionslets you accurately gate the “Widget State” tab to OPENAI_SDK tools while still always exposing Data and CSP (with a badge driven by violation count). This keeps the debug UI honest for MCP Apps without complicating the calling sites.Also applies to: 340-401, 438-450, 502-529
client/src/components/chat-v2/mcp-apps-renderer.tsx (1)
84-110: CSP mode handling, re‑fetch logic, and host context updates are coherentDeriving
cspModefrom the playground when active (with a permissive fallback), re‑fetching widget HTML whenevertoolState === "output-available"andloadedCspModeno longer matches, and clearing CSP violations on mode changes gives a clear, predictable lifecycle for MCP Apps CSP. The enrichedui/initializehost context andhost-context-changednotifications (theme, displayMode, locale, timeZone, maxHeight) are also well-scoped and avoid redundant updates. CSP violations being funneled into both the UI log andwidget-debug-storematches the intended debug UX.Also applies to: 121-189, 191-206, 232-242, 260-281, 283-325, 360-375, 517-561, 620-633
| const cspMeta = | ||
| '<meta http-equiv="Content-Security-Policy" content="' + | ||
| cspValue + | ||
| '">'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escape CSP value in meta tag attribute to prevent injection attacks.
The cspValue is concatenated directly into the HTML attribute without escaping. If the value ever contains a double-quote (e.g., from an unexpected domain format or sanitization bypass), it could break the attribute and potentially allow injection.
Apply this diff to safely escape the attribute value:
- function injectCSP(html, cspValue) {
- const cspMeta =
- '<meta http-equiv="Content-Security-Policy" content="' +
- cspValue +
- '">';
+ function injectCSP(html, cspValue) {
+ const escapedCsp = cspValue.replace(/"/g, '"');
+ const cspMeta =
+ '<meta http-equiv="Content-Security-Policy" content="' +
+ escapedCsp +
+ '">';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cspMeta = | |
| '<meta http-equiv="Content-Security-Policy" content="' + | |
| cspValue + | |
| '">'; | |
| function injectCSP(html, cspValue) { | |
| const escapedCsp = cspValue.replace(/"/g, '"'); | |
| const cspMeta = | |
| '<meta http-equiv="Content-Security-Policy" content="' + | |
| escapedCsp + | |
| '">'; |
🤖 Prompt for AI Agents
In server/routes/mcp/sandbox-proxy.html around lines 155 to 158, the CSP string
is concatenated directly into a double-quoted meta attribute which allows
injection if cspValue contains quotes or special chars; escape the attribute
value before inserting it (replace & < > " ' with their HTML entity equivalents
or use a safe attribute-escaping helper) and then build the meta tag with the
escaped value so the CSP cannot break out of the attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/components/chat-v2/mcp-apps-renderer.tsx (2)
217-285: Reset widget initialization state on CSP mode change.The widget HTML is correctly refetched when
cspModechanges (line 221), butisReadyremainstrue. This can cause premature notifications (e.g., host-context-changed) before the newly loaded widget callsui/initialize, leading to protocol violations or stale context references.Reset
isReadyand clearprevHostContextRefwhen initiating a refetch:const fetchWidgetHtml = async () => { try { + // Reset initialization state for new widget instance + setIsReady(false); + prevHostContextRef.current = null; + // Store widget data first (same pattern as openai.ts) const storeResponse = await fetch("/api/mcp/apps/widget/store", {
479-507: Validate inputSchema type before casting to object.Line 500 casts
toolMetadata?.inputSchematoobjectwithout verifying it's actually an object. IfinputSchemais a primitive or array, the cast succeeds silently but may cause downstream issues in the guest UI.Add a type guard to ensure safe object casting:
- inputSchema: (toolMetadata?.inputSchema as object) ?? { type: "object" }, + inputSchema: + typeof toolMetadata?.inputSchema === "object" && + toolMetadata.inputSchema !== null && + !Array.isArray(toolMetadata.inputSchema) + ? toolMetadata.inputSchema + : { type: "object" },
🧹 Nitpick comments (5)
client/src/components/chat-v2/thread.tsx (1)
462-462: Simplify optional type to idiomatic TypeScript.The type
uiType?: UIType | nullis redundant—optional parameters already permitundefined. Unlessnullcarries distinct semantic meaning from "not provided," prefer the simpleruiType?: UIType.Apply this diff:
- uiType?: UIType | null; + uiType?: UIType;client/src/stores/ui-playground-store.ts (2)
62-69: Consider makinggetInitialGlobalsSSR-safe while addingtimeZoneAdding
timeZonetoPlaygroundGlobalsand initializing it viaIntl.DateTimeFormat().resolvedOptions().timeZone || "UTC"is aligned with SEP‑1865 and should behave well in modern browsers and Node. However,locale: navigator.language || "en-US"(pre‑existing) and the newIntlcall both run at module initialization viagetInitialGlobals(), which will throw ifnavigator(or, in rarer environments,Intl) is missing.If this store is ever imported in non‑browser contexts (SSR, tests, Node tools), consider guarding both values:
const getInitialGlobals = (): PlaygroundGlobals => { const locale = typeof navigator !== "undefined" && navigator.language ? navigator.language : "en-US"; const timeZone = typeof Intl !== "undefined" && typeof Intl.DateTimeFormat === "function" ? Intl.DateTimeFormat().resolvedOptions().timeZone ?? "UTC" : "UTC"; return { theme: "dark", locale, timeZone, deviceType: "desktop", displayMode: "inline", userLocation: null, }; };That preserves current behavior in the browser while hardening the store for SSR/tests.
Also applies to: 161-168
111-119: CSP and protocol state wiring is coherent; verify reset semantics match UX expectationsSeparating
cspMode(ChatGPT Apps) frommcpAppsCspMode(MCP Apps) and introducingAppProtocol/selectedProtocolgives you a clear protocol‑aware CSP surface. Initializing both modes as"permissive"and resettingselectedProtocoltonullonsetSelectedToolmakes the protocol detection logic inUIPlaygroundTabstraightforward.One thing to double‑check is whether
reset()should also clearselectedProtocoland both CSP modes, or whether you’d prefer to persist protocol/CSP selections across tool reloads. Right nowreset()spreadsinitialState, so everything—includingmcpAppsCspModeandselectedProtocol—is reset. If the intended UX is “full playground reset,” this is perfect; if not, you might want to selectively preserve these fields similar toisSidebarVisibleandisPlaygroundActive.Also applies to: 152-155, 193-217, 321-324
client/src/components/ui-playground/UIPlaygroundTab.tsx (1)
208-246: Protocol detection logic is sound; consider guarding redundant updatesThe protocol detection effect correctly prefers the selected tool’s metadata and, when none is selected, infers a predominant protocol from all tools, falling back to
nullwhen the server exposes a mixed set. That matches the intended “ChatGPT vs MCP Apps” UX.You might consider a small optimization to avoid calling
setSelectedProtocolwhen the inferred value hasn’t changed (e.g., wrappingsetSelectedProtocolin a functional update that compares againststate.selectedProtocol). It’s not critical, but it can shave off a few unnecessary re‑renders when tools metadata changes frequently.client/src/components/ui-playground/PlaygroundMain.tsx (1)
519-688: Header controls work, but duplicated markup invites a small abstractionSplitting the header into
showChatGPTControlsandshowMCPAppsControlsbranches is an effective way to introduce MCP‑specific controls (timezone and MCP CSP) without disturbing the existing ChatGPT Apps experience. However, the two blocks now duplicate the device selector, locale selector, capability toggles, and Safe Area editor almost verbatim.To keep this maintainable, consider extracting a small internal component for the shared controls—for example:
function CommonDeviceControls(props: { deviceType: DeviceType; onDeviceTypeChange?: (type: DeviceType) => void; locale: string; onLocaleChange?: (locale: string) => void; capabilities: DeviceCapabilities; setCapabilities: (caps: Partial<DeviceCapabilities>) => void; }) { // existing device + locale + capabilities + SafeAreaEditor markup }You could then wrap it with thin protocol‑specific shells that add the CSP selector and, for MCP Apps, the timezone selector. This would reduce duplication and make future adjustments (like adding another capability toggle) less error‑prone.
Also applies to: 690-893
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/src/components/chat-v2/mcp-apps-renderer.tsx(19 hunks)client/src/components/chat-v2/thread.tsx(12 hunks)client/src/components/ui-playground/PlaygroundMain.tsx(7 hunks)client/src/components/ui-playground/UIPlaygroundTab.tsx(5 hunks)client/src/stores/ui-playground-store.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
client/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
client/**/components/**/*.{ts,tsx}: Use functional React components with React.FC typing
Implement strict TypeScript types for React component props with Props interface definitions
Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Use Radix UI components for Dialog, Dropdown menus, Form controls, and Tooltips
Design responsive layouts using Tailwind breakpoint system, grid layouts, flex containers, and container queries
Ensure accessibility compliance with ARIA attributes, keyboard navigation, focus management, and screen reader support
Files:
client/src/components/ui-playground/UIPlaygroundTab.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsxclient/src/components/chat-v2/thread.tsxclient/src/components/ui-playground/PlaygroundMain.tsx
client/**/{components,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Files:
client/src/components/ui-playground/UIPlaygroundTab.tsxclient/src/components/chat-v2/mcp-apps-renderer.tsxclient/src/components/chat-v2/thread.tsxclient/src/components/ui-playground/PlaygroundMain.tsx
client/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use Zustand for global state management with proper store creation, action definitions, selector optimization, and middleware usage
Files:
client/src/stores/ui-playground-store.ts
🧠 Learnings (11)
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/servers/**/*.{ts,tsx} : Provide transport protocol selection UI with protocol options, configuration forms, validation rules, and default presets
Applied to files:
client/src/components/ui-playground/UIPlaygroundTab.tsxclient/src/components/ui-playground/PlaygroundMain.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Implement message threading with thread components, collapse/expand functionality, navigation, and search capabilities
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/**/*.{ts,tsx} : Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Implement code highlighting with syntax detection, theme support, copy functionality, and line numbers in code components
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{stores/chat,hooks/llm}/**/*.{ts,tsx} : Manage context with window size tracking, token counting, context pruning, and state persistence for chat sessions
Applied to files:
client/src/components/chat-v2/thread.tsxclient/src/components/ui-playground/PlaygroundMain.tsxclient/src/stores/ui-playground-store.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/chat/**/*.{ts,tsx} : Build chat interface with message components, input handling, stream rendering, and history management
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/**/*.{ts,tsx} : Use Radix UI components for Dialog, Dropdown menus, Form controls, and Tooltips
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/stores/chat/**/*.{ts,tsx} : Implement AI model state handling including model selection state, generation parameters, stream management, and history persistence
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/{components,hooks}/**/*.{ts,tsx} : Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Applied to files:
client/src/components/chat-v2/thread.tsx
📚 Learning: 2025-11-25T19:59:26.547Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: server/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:59:26.547Z
Learning: Applies to server/**/types/mcp.ts : Define MCP protocol types in TypeScript type definition files
Applied to files:
client/src/stores/ui-playground-store.ts
📚 Learning: 2025-11-24T17:54:18.321Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: client/CLAUDE.md:0-0
Timestamp: 2025-11-24T17:54:18.321Z
Learning: Applies to client/**/components/**/*.{ts,tsx} : Design responsive layouts using Tailwind breakpoint system, grid layouts, flex containers, and container queries
Applied to files:
client/src/stores/ui-playground-store.ts
🧬 Code graph analysis (1)
client/src/components/ui-playground/PlaygroundMain.tsx (2)
client/src/stores/ui-playground-store.ts (3)
DeviceType(13-13)DEVICE_VIEWPORT_CONFIGS(16-20)CspMode(22-22)client/src/components/ui-playground/SafeAreaEditor.tsx (1)
SafeAreaEditor(146-237)
🔍 Remote MCP Context7
Summary of additional facts useful for reviewing and testing this PR
- Resolved repository: MCP Inspector documented at /modelcontextprotocol/inspector (best match; High reputation).
- Dev & run commands: repo README includes npm scripts for dev, build, start, lint, tests (npm run dev, npm run build, npm start, npm test, etc.) — useful to run the UI and server locally to exercise MCP Apps flows and sandbox proxy.
- MCP proxy endpoints and usage (relevant for end-to-end testing of widget fetch and sandbox flows):
- POST /mcp to initialize streamable-HTTP transport (used by inspector).
- GET /config and GET /health for proxy config/health checks.
- Examples show headers required (x-mcp-proxy-auth / session tokens) and initialization payload shape for MCP sessions. Use these when validating widget-content fetches and CSP query param handling.
- Transport examples: inspector supports Streamable HTTP and SSE transports; Streamable HTTP is recommended — useful when testing widget-content fetches with ?csp_mode= and validating server-side effectiveCspMode behavior.
- Build/dev tips: documented commands for hot-reload dev mode and co-development with SDK (npm run dev, npm run dev:sdk) — helpful to iterate on client/server changes (sandbox-proxy.html injection and CSP behavior).
Sources
- Repository resolution: Context7 library list for "MCPJam/inspector" → /modelcontextprotocol/inspector.
- Inspector docs (README + examples): build/run commands, proxy endpoints, transport usage, and examples.
⏰ 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: Cursor Bugbot
🔇 Additional comments (17)
client/src/components/chat-v2/thread.tsx (3)
14-14: Protocol-aware debug options with proper memoization.The
useMemohook correctly optimizes the debug options computation, preventing unnecessary recalculations when unrelated props change. The dependency array appropriately includesuiTypeand the violations count, ensuring the options update when the protocol changes or violations are added.Also applies to: 517-541
294-413: Protocol-aware rendering paths correctly implemented.The conditional propagation of display mode props to
ToolPartis intentionally varied based on widget type and state: MCP_UI resources, loading/error states, and the fallback case omit display controls, while fully loaded MCP_APPS and OPENAI_SDK widgets receive them. The Widget State tab is appropriately hidden for MCP_APPS (which don't support persistent widget state per SEP-1865).Also applies to: 529-531
554-678: Accessibility and UI component usage aligned with guidelines.The display mode controls use Radix UI Tooltips as specified in the coding guidelines. ARIA attributes (
aria-expanded,sr-onlyspans) properly support screen readers, and interactive elements use semanticbuttontags for keyboard navigation.Based on coding guidelines, Radix UI components are correctly employed for interactive controls.
client/src/stores/ui-playground-store.ts (1)
15-20: Shared device viewport config is a clean abstractionCentralizing the viewport sizes in
DEVICE_VIEWPORT_CONFIGSand exporting it for reuse in the renderer and playground keeps device-related layout logic consistent and avoids duplicated magic numbers. This is a solid structural improvement.client/src/components/ui-playground/UIPlaygroundTab.tsx (1)
92-99: Timezone wiring into globals and PlaygroundMain is clean
handleTimeZoneChangesimply delegates toupdateGlobal("timeZone", ...), and you thread bothtimeZone={globals.timeZone}andonTimeZoneChange={handleTimeZoneChange}intoPlaygroundMain. This keeps all host context state flowing from a single store and mirrors the existing locale wiring nicely.Also applies to: 341-343
client/src/components/ui-playground/PlaygroundMain.tsx (2)
68-76: Device configs and timezone presets are well-structuredBuilding
DEVICE_CONFIGSon top ofDEVICE_VIEWPORT_CONFIGSkeeps visual labels/icons layered cleanly over shared dimensions, and the use ofTIMEZONE_OPTIONSwith human‑friendly labels plus UTC offsets gives MCP Apps a practical, test‑oriented set of IANA zones. The fallback inSelectValuetotimeZonewhen a zone isn’t found ensures the UI remains robust even if the store holds an arbitrary IANA string.Also applies to: 98-120
295-309: Protocol-aware CSP mode selection behaves consistently across ChatGPT and MCP AppsThe pattern of reading both
cspModeandmcpAppsCspModefrom the store, then derivingactiveCspMode/setActiveCspModebased onselectedProtocol, cleanly separates ChatGPT Apps CSP from MCP Apps CSP while exposing a single selector in the ChatGPT path and an MCP‑specific selector in the MCP Apps path.A couple of points to verify:
- When
selectedProtocolflips betweennull/"openai-apps"and"mcp-apps", each protocol’s CSP setting remains independent, which seems intentional (different sandboxes, different defaults).- For MCP Apps, users always interact with the dedicated MCP selector (
mcpAppsCspMode), and the ChatGPT selector is hidden—so there’s no ambiguity about which CSP applies.If that matches the product spec, the wiring here looks solid.
Also applies to: 314-319, 595-631, 800-835
client/src/components/chat-v2/mcp-apps-renderer.tsx (10)
1-72: LGTM: Type definitions and imports are sound.The interface definitions clearly delineate controlled display mode semantics, and the new props (displayMode, onDisplayModeChange, onRequestFullscreen, onExitFullscreen, toolMetadata) align with React controlled component patterns.
96-159: LGTM: Playground integration with appropriate fallbacks.The CSP mode derivation and context propagation (locale, timeZone, platform, viewport, deviceCapabilities, safeAreaInsets) are correctly scoped to playground-active mode with sensible defaults for standalone usage.
161-190: LGTM: Controlled/uncontrolled display mode pattern is correctly implemented.The setDisplayMode callback appropriately handles both controlled and uncontrolled modes, and the fullscreen enter/exit detection logic correctly triggers parent callbacks based on mode transitions.
293-301: LGTM: CSP violations are correctly cleared when mode changes.The effect appropriately guards against initial mount and clears stale violation data when CSP mode transitions.
303-309: LGTM: Display mode synchronization respects controlled mode.The effect correctly syncs from playground store only when uncontrolled, preventing conflicts with parent-controlled display mode.
311-361: LGTM: Widget debug info is comprehensively maintained.Both initialization and update effects correctly track the expanded context fields (locale, timeZone, deviceCapabilities, safeAreaInsets) with appropriate dependencies.
406-444: LGTM: CSP violation handling is thorough.The flow correctly captures violation metadata, logs to UI store, persists to debug store, and emits console warnings for developer visibility.
655-738: LGTM: Granular host-context-changed logic correctly tracks deltas.The effect implements the SEP-1865 pattern of sending only changed fields, with proper object comparison for viewport, deviceCapabilities, and safeAreaInsets. The prevHostContextRef reset is addressed by the earlier recommendation at lines 217-285.
765-821: LGTM: Rendering logic handles all display modes correctly.The isPip calculation respects controlled mode, the close button correctly triggers both setDisplayMode and onExitPip callbacks, and the permissive flag is properly propagated to SandboxedIframe.
193-197: LGTM: maxHeight appropriately derived from device viewport.The calculation correctly aligns with DEVICE_VIEWPORT_CONFIGS and provides a sensible fallback for non-playground usage.
| sendResponse, | ||
| sendNotification, | ||
| addUiLog, | ||
| addCspViolation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing viewportHeight dependency in handleMessage callback
The handleMessage callback uses viewportHeight when building the viewport object in the ui/initialize response (line 489), but viewportHeight is missing from the useCallback dependency array. This causes the callback to capture a stale viewportHeight value when the device type changes, resulting in incorrect viewport dimensions being sent to the widget during initialization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
client/src/components/chat-v2/mcp-apps-renderer.tsx (2)
480-505: AddviewportHeighttohandleMessagedependencies to avoid stale initialize viewport
viewportHeightis used when responding to"ui/initialize":viewport: { width: viewportWidth, height: viewportHeight, maxHeight },but it is not included in the
useCallbackdependency array forhandleMessage. If the device type (and thusviewportHeight) changes, the callback will keep sending the old height in the initialize response.You’re already depending on
viewportWidth; mirroring that forviewportHeightfixes the captured state:[ themeMode, displayMode, contentHeight, maxHeight, locale, timeZone, platform, - viewportWidth, - deviceCapabilities, + viewportWidth, + viewportHeight, + deviceCapabilities, safeAreaInsets, toolName, toolMetadata, onCallTool, onSendFollowUp, serverId, toolCallId, toolInput, toolOutput, toolState, sendResponse, sendNotification, addUiLog, addCspViolation, ],Also applies to: 631-654
657-740: Reset host‑context baseline when widgets reload (e.g., after CSP changes)
prevHostContextRefandisReadyare never reset when the widget reloads (such as whencspModechanges and new HTML is fetched). After a reload:
- The iframe sends a fresh
ui/initializerequest and receives a fullhostContext.isReadyis alreadytrue, andprevHostContextRef.currentstill reflects the pre‑reload context.- The
host-context-changedeffect will therefore compute diffs against stale values, which may not match what the widget last saw inui/initialize.To make each reload establish a new baseline, you can reset
isReadyandprevHostContextRefwhen the CSP mode (or any reload trigger) changes. For example, placed after theprevHostContextRefdeclaration:+ useEffect(() => { + if (loadedCspMode !== null && loadedCspMode !== cspMode) { + // Force a new host-context baseline on next initialize + setIsReady(false); + prevHostContextRef.current = null; + } + }, [cspMode, loadedCspMode]);This ensures the next time the widget initializes, the first run of the host‑context effect re‑seeds
prevHostContextRefwith the new context instead of diffing against the previous session.
🧹 Nitpick comments (1)
client/src/components/chat-v2/mcp-apps-renderer.tsx (1)
39-63: Public API for display modes is clear; consider React.FC typing and stronger controlled‑mode typingThe
MCPAppsRendererPropsinterface and the newdisplayMode/ fullscreen callbacks make the external API readable and flexible. Two minor polish points:
- Coding guidelines for this repo prefer
React.FC-typed components; you might align with that via:-import function MCPAppsRenderer({ - ... -}: MCPAppsRendererProps) { +export const MCPAppsRenderer: React.FC<MCPAppsRendererProps> = ({ + ... +}) => {
- The comment says
onDisplayModeChangeis “required when displayMode is controlled”, but the type system can’t express that. If you want to enforce it, consider a discriminated union for controlled vs uncontrolled props; otherwise, ensure this contract is clearly documented where the component is used.Also applies to: 74-92
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/components/chat-v2/mcp-apps-renderer.tsx(20 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
client/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
client/**/components/**/*.{ts,tsx}: Use functional React components with React.FC typing
Implement strict TypeScript types for React component props with Props interface definitions
Follow React 19 patterns including hooks, Suspense boundaries, error boundaries, and concurrent features
Use Radix UI components for Dialog, Dropdown menus, Form controls, and Tooltips
Design responsive layouts using Tailwind breakpoint system, grid layouts, flex containers, and container queries
Ensure accessibility compliance with ARIA attributes, keyboard navigation, focus management, and screen reader support
Files:
client/src/components/chat-v2/mcp-apps-renderer.tsx
client/**/{components,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (client/CLAUDE.md)
Use React hooks (useState, useReducer) for local state management with proper effect cleanup
Files:
client/src/components/chat-v2/mcp-apps-renderer.tsx
🔍 Remote MCP Context7
Additional context for reviewing PR #1038
- Repository resolved as MCP Inspector (context7 ID: /modelcontextprotocol/inspector). Chosen because it exactly matches the repo name and has high reputation and coverage.
- Repo is a monorepo: client (React + Vite + TypeScript + Tailwind), server (Express + TypeScript), and CLI — changes span client and server so run/build/test guidance is relevant.
- Development / testing guidance:
- Use npm scripts (dev, build, start, test, e2e) and Prettier checks; PRs should pass existing CI checks and version consistency scripts. Running the UI + server locally is recommended to exercise sandbox proxy and widget flows.
- Inspector supports multiple transports (stdio, SSE, Streamable HTTP). Streamable HTTP is recommended for widget/content testing — important when validating server-side csp_mode query handling and end-to-end sandbox behavior.
- Config & security notes: inspector loads server configs from JSON for reproducible tests; built-in session token auth, origin validation, and CORS protections exist — verify new CSP/sandbox changes don’t disable or conflict with these protections.
Sources:
- Repository resolution and selection: Context7 resolve results for "MCPJam/inspector" — /modelcontextprotocol/inspector.
- README/CONTRIBUTING/AGENTS excerpts (development workflow, monorepo layout, dev/test commands, transports, security guidance).
⏰ 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: Cursor Bugbot
🔇 Additional comments (6)
client/src/components/chat-v2/mcp-apps-renderer.tsx (6)
96-160: Host context and viewport derivation are coherent and match the playground modelDeriving
cspMode,locale,timeZone,platform,viewportWidth/viewportHeight, andmaxHeightfrom the playground store (with sensible non‑playground defaults) yields a consistent host context for widgets and honors SEP‑1865 semantics. The separation between playground and non‑playground behavior is clear and looks correct.Also applies to: 193-197
204-206: CSP mode reload logic and violation clearing look correctThe
loadedCspModetracking in combination with:
if (widgetHtml && loadedCspMode === cspMode) return;in the fetch effect, andsetLoadedCspMode(cspMode)after a successful fetch, andclearCspViolations(toolCallId)whenloadedCspModeandcspModediverge,means widgets are re-fetched when the CSP mode changes and stale CSP violations are cleared alongside. This addresses the earlier concern where CSP mode changes didn’t affect already‑loaded widgets.
Also applies to: 217-285, 293-302
311-361: Widget debug store wiring cleanly surfaces extended globalsInitializing
widgetDebugInfowithglobalsand then keeping them in sync viasetWidgetGlobals—includingdisplayMode,maxHeight,locale,timeZone,deviceCapabilities, andsafeAreaInsets—gives the CSP/debug panels a faithful view of host context. The dependencies on the relevant fields ensure updates are propagated without redundant work.
403-445: CSP violation handling and size notification compatibility are well‑designedThe
handleMessagebranch forevent.data?.type === "mcp-apps:csp-violation":
- Logs via
addUiLog,- Records violations in the widget debug store, and
- Emits a concise
console.warn,which is a tight, developer‑friendly loop. Likewise, supporting both
"ui/notifications/size-changed"and"ui/notifications/size-change"keeps compatibility with older SDKs. Clamping height viaMath.min(sizeParams.height, maxHeight)is a sensible safety cap.Also applies to: 417-443, 614-623
767-796: PIP/fullscreen close behavior is consistent with displayMode semanticsUsing
setDisplayMode("inline")on the close button, then conditionally invokingonExitPipwhenisPipis true, and relying onsetDisplayMode’s internalonExitFullscreencallback when leaving fullscreen gives a clean, symmetric exit path for both modes. This meshes well with both controlled and uncontrolled displayMode usage.
804-817: Passingpermissivethrough toSandboxedIframecompletes the CSP wiringForwarding
widgetCspand thewidgetPermissiveflag intoSandboxedIframeties together the server‑computed CSP/permissive metadata with the sandboxed rendering layer. Combined with the earlier CSP mode handling, this gives a coherent end‑to‑end CSP story for MCP Apps.
Note
Adds MCP Apps playground controls (device/locale/timezone/CSP), expands host context and display-mode control, and implements CSP enforcement/violation reporting end-to-end.
client/src/components/chat-v2/mcp-apps-renderer.tsx)displayMode(inline/pip/fullscreen) with callbacks; syncs with playground; PiP/fullscreen close logic.cspMode,locale,timeZone,deviceCapabilities,safeAreaInsets, viewport (viaDEVICE_VIEWPORT_CONFIGS), and derivedplatform.ui/initializehost context (availableDisplayModes, viewport, locale/timeZone, platform/userAgent, deviceCapabilities, safeAreaInsets,toolInfo).ui/notifications/host-context-changedupdates; acceptsui/notifications/size-changed.csp_mode; handles returnedcspandpermissiveflags.widget-debug-store.client/src/components/chat-v2/thread.tsx)uiTypeand controlleddisplayModethroughToolPartandMCPAppsRenderer.client/src/components/ui-playground/PlaygroundMain.tsx,UIPlaygroundTab.tsx)selectedProtocol) and protocol-aware controls.DEVICE_VIEWPORT_CONFIGS.timeZone/displayModetoThread; toggles controls based on protocol.ui-playground-store: addsDEVICE_VIEWPORT_CONFIGS,timeZonein globals,mcpAppsCspMode,selectedProtocol, capability/safe-area actions; initial/default updates.widget-debug-store: preserve CSP info; addsclearCspViolationsAPI.server/routes/mcp/apps.ts: persistscspMode;widget-contentacceptscsp_modeand returnscspor permissive mode flag; logs effective CSP.SandboxedIframe: forwards CSP violation messages; sendspermissiveflag to proxy.server/routes/mcp/sandbox-proxy.html: injects CSP (incl. media-src), supports permissive mode, and posts CSP violation events to host.Written by Cursor Bugbot for commit 6ba509a. This will update automatically on new commits. Configure here.