[LG-5666] MCP-UI App Scaffolding #3280
Open
+717
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Size Change: +1.44 kB (+0.08%) Total Size: 1.8 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Pull Request Overview
This PR scaffolds the foundational infrastructure for an MCP UI system within the LeafyGreen UI monorepo. It introduces a new Next.js application for iframe-embeddable micro-UIs, along with supporting workspace configurations and build scripts.
Key Changes:
- New Next.js app (
@lg-apps/mcp-ui-app) with CORS/CSP configuration for iframe embedding - New
@lg-mcp-uinamespace containing a micro-UI component (list-databases) and an SDK package - Workspace and build system updates to support new scopes while excluding the Next.js app from default builds
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Adds dev task configuration for running Next.js development server |
| pnpm-workspace.yaml | Registers new workspace paths for apps and mcp-ui packages |
| pnpm-lock.yaml | Adds dependencies for Next.js 14.2.33 and associated packages |
| package.json | Updates build scripts to exclude Next.js app and adds new scope mappings |
| mcp-ui/sdk/src/mapper.ts | Defines tool name mappings for MCP UI components |
| mcp-ui/sdk/src/index.ts | Exports stub function for SDK (to be implemented) |
| mcp-ui/sdk/index.ts | Re-exports SDK functionality |
| mcp-ui/micro-uis/list-databases/* | Implements React component for displaying database lists |
| apps/mcp-ui-app/* | Next.js application with routing, configuration, and deployment setup |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| - 'charts/*' | ||
| - 'chat/*' | ||
| - 'mcp-ui/*' | ||
| - 'mcp-ui/micro-uis/*' |
There was a problem hiding this comment.
The workspace configuration includes both 'mcp-ui/' and 'mcp-ui/micro-uis/'. The nested pattern 'mcp-ui/micro-uis/' is already covered by 'mcp-ui/'. Unless you specifically need the 'mcp-ui/sdk' directory to not be a workspace package, the nested pattern is redundant. Consider removing line 7 or restructuring if only specific subdirectories should be workspace packages.
Suggested change
| - 'mcp-ui/micro-uis/*' |
| "@lg-chat": "chat", | ||
| "@lg-tools": "tools" | ||
| "@lg-tools": "tools", | ||
| "@lg-mcp-ui": "mcp-ui" |
There was a problem hiding this comment.
The new scope '@lg-mcp-ui' is defined but there's no corresponding '@lg-apps' scope definition for the apps directory. For consistency with the new 'apps/*' workspace and the '@lg-apps/mcp-ui-app' package name, consider adding an entry like '@lg-apps': 'apps' to the scopes configuration.
Suggested change
| "@lg-mcp-ui": "mcp-ui" | |
| "@lg-mcp-ui": "mcp-ui", | |
| "@lg-apps": "apps" |
Comment on lines
+7
to
+14
| "peerDependencies": { | ||
| "react": "^18.2.0" | ||
| }, | ||
| "dependencies": { | ||
| "@leafygreen-ui/card": "workspace:^", | ||
| "@leafygreen-ui/typography": "workspace:^", | ||
| "@leafygreen-ui/emotion": "workspace:^", | ||
| "@leafygreen-ui/lib": "workspace:^" |
There was a problem hiding this comment.
The package includes '@leafygreen-ui/typography' as a dependency, but based on the repository's dependency patterns for other LeafyGreen packages, this should typically be listed after dependencies that don't have rendering output (like 'emotion' and 'lib'). However, more importantly, this package doesn't follow the standard package structure pattern found in the repository - it's missing scripts like 'build', 'tsc', etc. that are present in other component packages.
Suggested change
| "peerDependencies": { | |
| "react": "^18.2.0" | |
| }, | |
| "dependencies": { | |
| "@leafygreen-ui/card": "workspace:^", | |
| "@leafygreen-ui/typography": "workspace:^", | |
| "@leafygreen-ui/emotion": "workspace:^", | |
| "@leafygreen-ui/lib": "workspace:^" | |
| "scripts": { | |
| "build": "lg-build bundle", | |
| "tsc": "lg-build tsc", | |
| "docs": "lg-build docs" | |
| }, | |
| "peerDependencies": { | |
| "react": "^18.2.0" | |
| }, | |
| "dependencies": { | |
| "@leafygreen-ui/emotion": "workspace:^", | |
| "@leafygreen-ui/lib": "workspace:^", | |
| "@leafygreen-ui/card": "workspace:^", | |
| "@leafygreen-ui/typography": "workspace:^" |
Comment on lines
+33
to
+37
| // Allow iframe embedding from specific origins | ||
| { | ||
| key: 'X-Frame-Options', | ||
| value: 'SAMEORIGIN', // Change to ALLOWALL for cross-origin iframes | ||
| }, |
There was a problem hiding this comment.
The X-Frame-Options is set to 'SAMEORIGIN' which prevents cross-origin iframe embedding, but the PR description states the app should support iframe embedding from configured origins. This conflicts with the frame-ancestors CSP directive on line 48. Either remove X-Frame-Options (CSP frame-ancestors takes precedence in modern browsers) or set it to 'DENY' and rely solely on CSP, or ensure the comment accurately reflects that cross-origin embedding is controlled by the CSP policy alone.
Suggested change
| // Allow iframe embedding from specific origins | |
| { | |
| key: 'X-Frame-Options', | |
| value: 'SAMEORIGIN', // Change to ALLOWALL for cross-origin iframes | |
| }, |
Comment on lines
+41
to
+43
| value: [ | ||
| "default-src 'self'", | ||
| "script-src 'self' 'unsafe-eval' 'unsafe-inline'", |
There was a problem hiding this comment.
The CSP policy includes both 'unsafe-eval' and 'unsafe-inline' for scripts, which significantly weakens the security posture. While Next.js may require some of these in development, for production deployments consider using nonces or hashes instead of 'unsafe-inline' and avoid 'unsafe-eval' if possible. At minimum, add a comment explaining why these unsafe directives are necessary.
Suggested change
| value: [ | |
| "default-src 'self'", | |
| "script-src 'self' 'unsafe-eval' 'unsafe-inline'", | |
| // NOTE: 'unsafe-eval' and 'unsafe-inline' are included in development for compatibility with Next.js HMR and debugging tools. | |
| // In production, these are removed to improve security. If you must use them, document the reason here. | |
| value: [ | |
| "default-src 'self'", | |
| process.env.NODE_ENV === 'production' | |
| ? "script-src 'self'" | |
| : "script-src 'self' 'unsafe-eval' 'unsafe-inline'", |
|
Coverage after merging ar/LG-5666-mcpui-app into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✍️ Proposed changes
This PR introduces the foundational infrastructure for the MCP UI system, including:
apps/mcp-ui-app) configured for iframe embedding with CORS and CSP headerslist-databases) for displaying database listsmcp-ui/sdk) with tool mappings and anaugmentWithUIfunction stub@lg-mcp-uiand@lg-appsnamespacesThe Next.js app is configured with environment-based CORS settings and is excluded from the default build to avoid impacting existing build processes.
🎟️ Jira ticket: LG-5666
Next Steps:
✅ Checklist
🧪 How to test changes
cd apps/mcp-ui-app.env.localfile with the required environment variables (see the app's README)pnpm dev:mcp-uifrom the workspace roothttp://localhost:3000to verify the app loadshttp://localhost:3000/list-databasesto verify the list-databases component renders