fix(mcp): bundle @uppy into self-contained MCP app bundles

internal/mcpapp/mcpapp_test.go

@@ -1,6 +1,8 @@
 package mcpapp
 
 import (
+	"regexp"
+	"sort"
 	"strings"
 	"testing"
 )
@@ -42,18 +44,118 @@ func TestMcpAppThemeCSSEmbedded(t *testing.T) {
 	}
 }
 
-// TestAppModuleJSEmbedded pins that each app's self-contained bundle is
-// embedded and inlines into the served document. A missing/empty bundle (JS
-// not built before Go) panics, so a passing test also proves `pnpm build` ran.
+// fromSpecifierRe matches the module-specifier string in `import ... from
+// "spec"`, side-effect `import "spec"`, and dynamic `import("spec")` forms.
+// Minified bundles drop the space around the keyword/specifier, so both
+// `from "x"` and `from"x"` (and `import"x"` / `import("x")`) must match.
+var fromSpecifierRe = regexp.MustCompile(`from\s*["']([^"']+)["']|(?:^|[;)\]}])import\s*["']([^"']+)["']|(?:^|[;)\]}])import\s*\(\s*["']([^"']+)["']`)
+
+// bareModuleSpecifiers returns any module specifiers in an inline-ready bundle
+// that the browser cannot resolve on its own: bare package specifiers (e.g.
+// "@uppy/core") that do NOT start with ".", "/", or a URL scheme. The sandboxed
+// ui:// iframe serves each app as a single inline <script type="module"> with
+// no importer and no node_modules, so any such specifier throws
+// "Failed to resolve module specifier ..." and kills the app at load time —
+// exactly the @uppy/core regression this guards against.
+func bareModuleSpecifiers(src string) []string {
+	seen := map[string]bool{}
+	for _, m := range fromSpecifierRe.FindAllStringSubmatch(src, -1) {
+		spec := m[1]
+		if spec == "" {
+			spec = m[2]
+		}
+		if spec == "" {
+			spec = m[3]
+		}
+		if spec == "" {
+			continue
+		}
+		if strings.HasPrefix(spec, ".") || strings.HasPrefix(spec, "/") {
+			continue
+		}
+		if isResolvableURL(spec) {
+			continue
+		}
+		seen[spec] = true
+	}
+	out := make([]string, 0, len(seen))
+	for s := range seen {
+		out = append(out, s)
+	}
+	sort.Strings(out)
+	return out
+}
+
+// isResolvableURL reports whether a specifier is an absolute URL the browser
+// can fetch directly (e.g. https://... or //cdn...), which is fine inline.
+func isResolvableURL(spec string) bool {
+	for _, p := range []string{"https://", "http://", "//", "data:", "blob:"} {
+		if strings.HasPrefix(spec, p) {
+			return true
+		}
+	}
+	return false
+}
+
+// TestBareModuleSpecifiers pins the self-containment guard itself: it must flag
+// bare package specifiers (including the minified no-space forms and the exact
+// @uppy/* regression), while ignoring relative/absolute URLs it can resolve.
+func TestBareModuleSpecifiers(t *testing.T) {
+	good := []string{
+		`const a = 1;`,
+		`import "./local.js";`,
+		`import x from "/abs/mod.js";`,
+		`import x from "https://cdn.example/lib.js";`,
+	}
+	bad := []string{
+		`import e from"@uppy/core";`,
+		`import t from "@uppy/xhr-upload";`,
+		`import "zod";`,
+		`import { x } from "@modelcontextprotocol/sdk/client.js";`,
+		`import("@uppy/core");`,
+		`import ("@uppy/xhr-upload");`,
+	}
+	for _, s := range good {
+		if got := bareModuleSpecifiers(s); len(got) != 0 {
+			t.Errorf("bareModuleSpecifiers(%q) = %v, want []", s, got)
+		}
+	}
+	for _, s := range bad {
+		got := bareModuleSpecifiers(s)
+		if len(got) == 0 {
+			t.Errorf("bareModuleSpecifiers(%q) = [] , want a flagged specifier", s)
+		}
+	}
+}
+
+// TestAppModuleJSEmbedded pins that EVERY app's self-contained bundle is
+// embedded and inlines into the served document with zero bare module imports.
+// A missing/empty bundle (JS not built before Go) panics, so a passing test
+// also proves `pnpm build` ran; a residual bare import (e.g. "@uppy/core" or
+// "@uppy/xhr-upload" leaking out of the tsdown build) fails self-containment
+// and would crash every app that ships it in a browser host.
 func TestAppModuleJSEmbedded(t *testing.T) {
-	for _, app := range []string{"pin", "vault-create", "vault-restore", "auth-sso", "vault-browser", "pin-list", "auth-status"} {
+	for app, file := range bundleNames {
+		_ = app // key used only for diagnostic clarity below
+		_ = file
+	}
+	// Cover every app the Go layer embeds, not just a subset. Historically two
+	// upload bundles shipped `import ... from "@uppy/*"` bare imports while the
+	// subset of apps tested here passed, so the upload apps were the last thing
+	// you'd expect to catch this.
+	apps := make([]string, 0, len(bundleNames))
+	for app := range bundleNames {
+		apps = append(apps, app)
+	}
+	sort.Strings(apps)
+	for _, app := range apps {
 		src := AppModuleJS(app)
 		if strings.TrimSpace(src) == "" {
 			t.Fatalf("app bundle %q is empty", app)
 		}
-		// The bundle must be inline-module-ready: no unresolved file imports.
-		if strings.Contains(src, "import ") {
-			t.Errorf("app bundle %q is not self-contained (contains import)", app)
+		if bare := bareModuleSpecifiers(src); len(bare) > 0 {
+			t.Errorf("app bundle %q is not inline-module-ready (bare imports the browser cannot resolve: %v). "+
+				"Run `pnpm build` (packages/apps) — a dependency missing from alwaysBundle stays external.", app, bare)
 		}
 	}
 }

packages/apps/scripts/build-apps.mjs

@@ -24,7 +24,22 @@ for (const app of APPS) {
     clean: false, // don't wipe other apps' outputs
     outDir: "dist",
     deps: {
-      alwaysBundle: ["@modelcontextprotocol/ext-apps", "@modelcontextprotocol/sdk", "zod", "robot3"],
+      // Force-bundle EVERY runtime dependency. The bundle is served as a single
+      // inline <script type="module"> in a sandboxed iframe that cannot resolve
+      // bare file imports, so nothing may be left external. @uppy/core and
+      // @uppy/xhr-upload power the upload apps' out-of-band XHR uploader; if
+      // they are not inlined they'd ship as `import ... from "@uppy/core"`,
+      // which the browser cannot resolve and kills the app ("Failed to resolve
+      // module specifier"). Always keep this list in sync with the runtime
+      // dependencies in package.json.
+      alwaysBundle: [
+        "@modelcontextprotocol/ext-apps",
+        "@modelcontextprotocol/sdk",
+        "zod",
+        "robot3",
+        "@uppy/core",
+        "@uppy/xhr-upload",
+      ],
       onlyBundle: false,
     },
   });

tests/sunpeak/tests/inspector-apps.test.ts

@@ -1,4 +1,36 @@
 import { test, expect } from 'sunpeak/test';
+import type { FrameLocator } from '@playwright/test';
+
+/**
+ * The upload apps (Upload to IPFS / Upload to Vault) ship Uppy's XHR uploader
+ * inlined into a single self-contained ESM bundle. The sandboxed ui:// iframe
+ * serves each bundle as one inline <script type="module"> with no importer, so
+ * NO bare module specifier may survive the build — a leaked `import ... from
+ * "@uppy/core"` throws "Failed to resolve module specifier" and kills the app
+ * before it boots (a real regression that hit Claude's host). These tests
+ * render the upload apps in the real browser and assert the module actually
+ * EXECUTED: the shell injects `window.__PINNER_CLI_VERSION__` as the first
+ * statement of the same module script, and ES module instantiation resolves
+ * every import before running any statement. If an import could not resolve,
+ * the version global would never be set and the app body stays inert — which
+ * is exactly the failure mode we guard against.
+ */
+
+/** The version global the shell injects as the first statement of each app's module. */
+const VERSION_GLOBAL = '__PINNER_CLI_VERSION__';
+
+// bootedVersion reads the version global in the app (sandboxed iframe) window,
+// evaluating inside that frame. A non-empty string proves the module graph
+// instantiated (all imports resolved) and then executed its first statement.
+async function appVersion(app: FrameLocator): Promise<unknown> {
+  return app
+    .locator('body')
+    .evaluate((el, globalName) => {
+      const doc = el.ownerDocument;
+      const win = doc?.defaultView as (Window & Record<string, unknown>) | null;
+      return win ? win[globalName] : undefined;
+    }, VERSION_GLOBAL);
+}
 
 /**
  * Host-iframe rendering of the pinner ui:// MCP Apps via the sunpeak
@@ -21,3 +53,32 @@ test('auth_sso app renders its sign-in view inside the host iframe', async ({ in
   const body = await app.locator('body').innerText();
   expect(body).toContain('Sign In');
 });
+
+// assertUploadAppBoots renders an upload tool's ui:// view and verifies the app
+// actually booted: the static HTML shell is server-rendered (it would render
+// even if the module failed), so the real signal is that the inline module ran
+// (window.__PINNER_CLI_VERSION__ is set). A leaked bare import — e.g. Uppy
+// left external as `import ... from "@uppy/core"` — fails module instantiation,
+// leaving the global unset and the app's wiring inert.
+async function assertUploadAppBoots(tool: string, heading: string, inspector: { renderTool: (n: string, i: unknown) => Promise<{ app(): FrameLocator }> }) {
+  const result = await inspector.renderTool(tool, {});
+  const app = result.app();
+
+  // The static shell still renders its heading even when the module is broken,
+  // so presence alone is necessary but not sufficient.
+  const body = await app.locator('body').innerText();
+  expect(body).toContain(heading);
+
+  // The module must have instantiated and executed. This is the assertion that
+  // catches an unresolved bare import (the @uppy/core regression).
+  const version = await appVersion(app);
+  expect(version).toEqual(expect.stringMatching(/^\d+\.\d+\.\d+/));
+}
+
+test('upload_file app boots (ipfs-upload bundle has no unmet imports)', async ({ inspector }) => {
+  await assertUploadAppBoots('upload_file', 'Upload to IPFS', inspector);
+});
+
+test('vault_put_file app boots (vault-upload bundle has no unmet imports)', async ({ inspector }) => {
+  await assertUploadAppBoots('vault_put_file', 'Upload to Vault', inspector);
+});