fix(mcp): bundle @uppy into self-contained MCP app bundles - #451
Merged
Conversation
The upload MCP apps (upload_file, vault_put_file) depend on @uppy/core and @uppy/xhr-upload, but these were not in the tsdown alwaysBundle list, so they were externalized as bare module specifiers (e.g. import ... from "@uppy/core") that the host's sandboxed app iframe cannot resolve. This caused a 'Failed to resolve module specifier "@uppy/core"' crash when rendering the apps in Claude. Fix by forcing @uppy/* into the self-contained bundle, and add regression coverage at both layers: - Go: mcpapp_test TestAppModuleJSEmbedded now scans all embedded app bundles for bare module specifiers (previously only a 7-app subset that missed the upload apps), plus a self-check for the detector. - Browser: sunpeak inspector tests render upload_file and vault_put_file in real Chromium across ChatGPT and Claude hosts, asserting the bundle module actually executed via the window.__PINNER_CLI_VERSION__ global (which is only set after all static imports resolve).
This comment has been minimized.
This comment has been minimized.
Code Coverage ReportTotal Coverage: 51.7% Generated from commit: 67f20fe |
The MCP app self-containment guard only matched static import-from and side-effect import forms. A bare specifier introduced via a dynamic import call would slip past CI and throw the same Failed to resolve module specifier error when the sandboxed inline module evaluates it. Extend the regex to match the dynamic form (tolerating whitespace around the parens) and add bad-vector coverage for it.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
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
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.
Summary
Fixes Claude hitting
Failed to resolve module specifier "@uppy/core"when rendering the MCP Apps (ui://views) forupload_fileandvault_put_file.Root cause
The upload apps depend on
@uppy/core+@uppy/xhr-uploadfor the out-of-band XHR uploader, but these weren't in thealwaysBundlelist ofpackages/apps/scripts/build-apps.mjs. tsdown therefore externalized them into bare module specifiers (import ... from "@uppy/core") that survive into the served bundle. Host apps are sandboxed so those bare imports cannot resolve, and the iframe fails to render.Fix
Add
@uppy/coreand@uppy/xhr-uploadtoalwaysBundle, forcing them into the self-contained inline-module bundle. Rebuilt all 14 bundles and resynced tointernal/mcpapp/appsassets/dist/(gitignored; CI regenerates via the js-bundle step).Regression coverage
internal/mcpapp/mcpapp_test.go):TestAppModuleJSEmbeddednow scans all embedded app bundles (previously only a 7-app subset that missed the broken upload apps) for bare module specifiers, with aTestBareModuleSpecifiersself-check for the detector.tests/sunpeak/tests/inspector-apps.test.ts): newupload_fileandvault_put_filetests render the apps in real Chromium across ChatGPT and Claude hosts. They assert the module actually executed viawindow.__PINNER_CLI_VERSION__(set as the first statement of the same module script — only present after all static imports resolve), so an unmet import fails the test.Verification
go test ./internal/mcpapp/...— pass@uppy/coreimport makes theupload_filetest fail, proving coverage catches the exact regressionFiles changed
packages/apps/scripts/build-apps.mjsinternal/mcpapp/mcpapp_test.gotests/sunpeak/tests/inspector-apps.test.tsSummary
This pull request fixes a critical issue where the MCP apps' self-contained JavaScript bundles contained bare module imports (specifically from
@uppy/coreand@uppy/xhr-upload) that the browser cannot resolve when served as inline<script type="module">tags in a sandboxed iframe. This caused the upload apps (Upload to IPFS / Upload to Vault) to fail at load time with "Failed to resolve module specifier" errors.Changes
Build Configuration (
packages/apps/scripts/build-apps.mjs)@uppy/coreand@uppy/xhr-uploadto thealwaysBundlelist in the tsdown build configurationTest Enhancements (
internal/mcpapp/mcpapp_test.go)bareModuleSpecifiersfunction that detects bare module specifiers (imports not starting with.,/, or a URL scheme) in the bundle source codeTestBareModuleSpecifiersto validate the detection logic against good and bad import patternsTestAppModuleJSEmbeddedto check ALL apps (not just a subset) for bare imports that would fail in a browser environment@uppy/*regression that previously slipped through because the old test only checked a subset of appsIntegration Tests (
tests/sunpeak/tests/inspector-apps.test.ts)__PINNER_CLI_VERSION__global variable that's injected as the first statement of the moduleupload_file(Upload to IPFS) andvault_put_file(Upload to Vault) appsRoot Cause
The
tsdownbuild configuration did not include Uppy packages in thealwaysBundlelist, so they were left as external dependencies that resolved to bare imports (e.g.,import from "@uppy/core"). When served as an inline module in a sandboxed iframe with no node_modules available, these imports could not be resolved, crashing the apps.