-
Notifications
You must be signed in to change notification settings - Fork 322
feat: add adapters infra (appssdk) #123
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
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.
Pull Request Overview
This PR introduces an adapters infrastructure for MCP-UI with Apps SDK as the first implementation. The adapters enable MCP-UI widgets to work in platform-specific environments (like ChatGPT) by translating the standard MCP-UI postMessage protocol to platform-specific APIs.
Key changes include:
- Added comprehensive adapter configuration types and infrastructure
- Implemented Apps SDK adapter with runtime bundling system
- Added adapter integration to the
createUIResourcefunction - Created extensive test coverage and example applications
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdks/typescript/server/src/utils.ts | Added wrapHtmlWithAdapters function and adapter imports |
| sdks/typescript/server/src/types.ts | Added AdaptersConfig and AppsSdkAdapterOptions type definitions |
| sdks/typescript/server/src/index.ts | Integrated adapter wrapping into createUIResource and exported adapter types |
| sdks/typescript/server/src/adapters/index.ts | Main adapters module export |
| sdks/typescript/server/src/adapters/appssdk/types.ts | Comprehensive type definitions for Apps SDK adapter |
| sdks/typescript/server/src/adapters/appssdk/index.ts | Apps SDK adapter public exports |
| sdks/typescript/server/src/adapters/appssdk/adapter.ts | Adapter script generation and configuration injection |
| sdks/typescript/server/src/adapters/appssdk/adapter-runtime.ts | Complete Apps SDK adapter implementation with message handling |
| sdks/typescript/server/src/adapters/appssdk/adapter-runtime.bundled.ts | Auto-generated bundled runtime script |
| sdks/typescript/server/src/adapters/appssdk/README.md | Comprehensive documentation for the adapter architecture |
| sdks/typescript/server/src/tests/adapters/appssdk-adapter.test.ts | Extensive test suite for adapter functionality |
| sdks/typescript/server/src/tests/adapters/adapter-integration.test.ts | Integration tests for adapter with createUIResource |
| sdks/typescript/server/scripts/bundle-adapter.js | Build script for bundling TypeScript adapter to injectable JavaScript |
| sdks/typescript/server/package.json | Added esbuild dependency and bundle scripts |
| sdks/typescript/server/README.md | Documentation for platform adapters usage |
| examples/appssdk-adapter-demo/example.ts | Complete example implementations showing adapter usage |
| examples/appssdk-adapter-demo/README.md | Example documentation and usage guide |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
sdks/typescript/server/src/adapters/appssdk/adapter-runtime.ts:1
- The event listener callback function is unnecessarily cast to
EventListener. The arrow function should be typed properly or the cast should be removed if the types match.
/**
| @@ -0,0 +1,375 @@ | |||
| // This file is auto-generated by scripts/bundle-adapter.js | |||
| // Do not edit directly - modify adapter-runtime.ts instead | |||
Copilot
AI
Oct 10, 2025
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.
[nitpick] Consider adding a build timestamp or hash to the auto-generated file header to help track when it was last built and detect stale builds.
| // Do not edit directly - modify adapter-runtime.ts instead | |
| // Do not edit directly - modify adapter-runtime.ts instead | |
| // Build timestamp: 2024-06-10T00:00:00Z | |
| // Build hash: <BUILD_HASH_PLACEHOLDER> |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Deploying mcp-ui with
|
| Latest commit: |
85af176
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://91e038c9.mcp-ui.pages.dev |
| Branch Preview URL: | https://feat-adapters.mcp-ui.pages.dev |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
examples/appssdk-adapter-demo/example.ts:1
- Using innerHTML with template literals containing user data can lead to XSS vulnerabilities. Consider using textContent or properly sanitizing the data before inserting it into the DOM.
/**
| return htmlContent; | ||
| } | ||
|
|
||
| const wrappedHtml = htmlContent; |
Copilot
AI
Oct 10, 2025
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.
The variable wrappedHtml is assigned but never modified. Consider removing this variable and using htmlContent directly in the return statements to improve code clarity.
| if (this.originalPostMessage) { | ||
| try { | ||
| const originalPostMessage = this.originalPostMessage; | ||
| (window.parent as Window & { postMessage: typeof originalPostMessage }).postMessage = originalPostMessage; |
Copilot
AI
Oct 10, 2025
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.
The type assertion is complex and duplicated. Consider extracting this to a variable or helper function for better readability and maintainability.
|
|
||
| try { | ||
| // Replace parent.postMessage with our interceptor | ||
| (window.parent as Window & { postMessage: typeof postMessageInterceptor }).postMessage = postMessageInterceptor; |
Copilot
AI
Oct 10, 2025
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.
This type assertion is similar to the one in the uninstall method. Consider creating a helper function to handle these postMessage assignments safely.
|
Replaced by #127 |
Initial adapters infra with AppsSDK as a first use case.
Based on the work of @yannj-fr.