From 25f9ca9a8c959fe3f9e918a60ec3f39f25eaa72e Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Mon, 4 May 2026 17:13:16 -0500
Subject: [PATCH 01/21] feat: upload and install custom .mcpb bundles from web
 UI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds end-to-end flow for uploading .mcpb bundles not on the mpak
registry. REST endpoint validates via SDK's validateMcpb(), manage_app
tool handles installation. Uploaded bundles are treated identically to
registry bundles — no separate "sideloaded" distinction.

- POST /v1/bundles/upload: multipart upload + validation
- manage_app tool: accepts optional path for file-based install
- startBundleSource: routes .mcpb archives through SDK prepareServer
- About tab: Upload button on Installed Bundles section

Closes #169

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/api/app.ts                      |  2 +
 src/api/handlers.ts                 | 81 ++++++++++++++++++++++++++
 src/api/routes/bundles.ts           | 21 +++++++
 src/bundles/startup.ts              | 48 ++++++++++++++++
 src/tools/system-tools.ts           | 60 +++++++++++++------
 web/src/api/client.ts               | 53 +++++++++++++++++
 web/src/pages/settings/AboutTab.tsx | 89 ++++++++++++++++++++++++++---
 7 files changed, 329 insertions(+), 25 deletions(-)
 create mode 100644 src/api/routes/bundles.ts

diff --git a/src/api/app.ts b/src/api/app.ts
index 7f2f93b5..a13a5d74 100644
--- a/src/api/app.ts
+++ b/src/api/app.ts
@@ -11,6 +11,7 @@ import { healthRoutes } from "./routes/health.ts";
 import { mcpRoutes } from "./routes/mcp.ts";
 import { mcpAuthRoutes } from "./routes/mcp-auth.ts";
 import { proxyRoutes } from "./routes/proxy.ts";
+import { bundleRoutes } from "./routes/bundles.ts";
 import { resourceRoutes } from "./routes/resources.ts";
 import { toolRoutes } from "./routes/tools.ts";
 import { wellKnownRoutes } from "./routes/well-known.ts";
@@ -56,6 +57,7 @@ export function createApp(
   app.route("/", chatRoutes(ctx));
   app.route("/", toolRoutes(ctx));
   app.route("/", resourceRoutes(ctx));
+  app.route("/", bundleRoutes(ctx));
   app.route("/", eventRoutes(ctx));
   app.route("/", conversationEventRoutes(ctx));
 
diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index b7ea1e32..0dc56b20 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1374,3 +1374,84 @@ export async function handleResourceUpload(
   }
   return json({ files: entries, ...(errors.length > 0 ? { errors } : {}) });
 }
+
+// ---------------------------------------------------------------------------
+// Bundle upload
+// ---------------------------------------------------------------------------
+
+export async function handleBundleUpload(
+  raw: Request,
+  runtime: Runtime,
+  workspaceId: string,
+): Promise<Response> {
+  let formData: FormData;
+  try {
+    formData = await raw.formData();
+  } catch {
+    return apiError(400, "bad_request", "Expected multipart/form-data");
+  }
+
+  const fileEntry = formData.get("file") ?? formData.get("bundle");
+  if (!fileEntry || typeof fileEntry === "string") {
+    return apiError(400, "bad_request", "No bundle file in request (use the 'file' or 'bundle' field)");
+  }
+
+  const entry = fileEntry as unknown as {
+    arrayBuffer(): Promise<ArrayBuffer>;
+    name?: string;
+    type?: string;
+  };
+  if (typeof entry.arrayBuffer !== "function") {
+    return apiError(400, "bad_request", "Malformed file entry in multipart body");
+  }
+
+  const filename = entry.name || "bundle.mcpb";
+  if (!filename.endsWith(".mcpb")) {
+    return apiError(400, "bad_request", "File must have .mcpb extension");
+  }
+
+  const data = Buffer.from(await entry.arrayBuffer());
+  if (data.length === 0) {
+    return apiError(400, "bad_request", "Uploaded file is empty");
+  }
+
+  // Save to workspace bundles directory
+  const { existsSync, mkdirSync, writeFileSync } = await import("node:fs");
+  const { join: joinPath } = await import("node:path");
+
+  const bundlesDir = joinPath(runtime.getWorkspaceScopedDir(workspaceId), "bundles");
+  mkdirSync(bundlesDir, { recursive: true });
+
+  const safeName = sanitizeFilename(filename);
+  const bundlePath = joinPath(bundlesDir, safeName);
+  writeFileSync(bundlePath, data, { mode: 0o600 });
+
+  // Validate via mpak SDK
+  const { validateMcpb } = await import("@nimblebrain/mpak-sdk");
+  const result = await validateMcpb(bundlePath);
+
+  if (!result.valid) {
+    // Clean up invalid bundle
+    const { unlinkSync } = await import("node:fs");
+    try {
+      unlinkSync(bundlePath);
+    } catch {
+      // best-effort cleanup
+    }
+    return apiError(400, "invalid_bundle", "Bundle validation failed", {
+      errors: result.errors,
+    });
+  }
+
+  return json({
+    path: bundlePath,
+    manifest: {
+      name: result.manifest.name,
+      version: result.manifest.version,
+      description: result.manifest.description,
+      display_name: result.manifest.display_name ?? null,
+      server_type: result.manifest.server.type,
+      tools: result.manifest.tools ?? [],
+    },
+  });
+}
diff --git a/src/api/routes/bundles.ts b/src/api/routes/bundles.ts
new file mode 100644
index 00000000..a04526a1
--- /dev/null
+++ b/src/api/routes/bundles.ts
@@ -0,0 +1,21 @@
+import { Hono } from "hono";
+import { handleBundleUpload } from "../handlers.ts";
+import { requireAuth } from "../middleware/auth.ts";
+import { bodyLimit } from "../middleware/body-limit.ts";
+import { errorLog } from "../middleware/error-log.ts";
+import { requireWorkspace } from "../middleware/workspace.ts";
+import type { AppContext, AppEnv } from "../types.ts";
+
+const MAX_BUNDLE_SIZE = 200 * 1024 * 1024; // 200 MB
+
+export function bundleRoutes(ctx: AppContext) {
+  return new Hono<AppEnv>()
+    .use("*", requireAuth(ctx.authOptions))
+    .use("*", requireWorkspace(ctx.workspaceStore))
+    .use("*", errorLog(ctx))
+    .post(
+      "/v1/bundles/upload",
+      bodyLimit(1_048_576, { multipart: MAX_BUNDLE_SIZE }),
+      (c) => handleBundleUpload(c.req.raw, ctx.runtime, c.var.workspaceId),
+    );
+}
diff --git a/src/bundles/startup.ts b/src/bundles/startup.ts
index d9f2b32d..02ff65c3 100644
--- a/src/bundles/startup.ts
+++ b/src/bundles/startup.ts
@@ -485,6 +485,54 @@ export async function startBundleSource(
       },
       eventSink,
     );
+  } else if ("path" in ref && ref.path.endsWith(".mcpb")) {
+    // .mcpb archive — use the SDK to extract and prepare
+    const mpakHome = process.env.MPAK_HOME ?? join(homedir(), ".mpak");
+    const mpak = getMpak(mpakHome);
+    const nbWorkDir = opts?.workDir ?? process.env.NB_WORK_DIR ?? join(homedir(), ".nimblebrain");
+
+    const server = await mpak.prepareServer(
+      { local: ref.path },
+      { workspaceDir: opts?.dataDir ?? nbWorkDir },
+    );
+
+    const serverName = deriveServerName(server.name);
+    validateServerName(serverName);
+
+    // Read manifest from the extracted cache dir
+    const manifestPath = join(server.cwd, "manifest.json");
+    const rawManifest = JSON.parse(readFileSync(manifestPath, "utf-8"));
+    const manifestResult = validateManifest(rawManifest);
+    if (manifestResult.valid && manifestResult.manifest) {
+      manifest = manifestResult.manifest;
+      meta = extractBundleMeta(rawManifest);
+    }
+
+    const platformEnv = buildPlatformEnv({
+      workspaceId: opts?.wsId,
+      serverName,
+      manifestMeta: (manifest?._meta ?? undefined) as Record<string, unknown> | undefined,
+      publicOrigin: resolvePublicOrigin(),
+    });
+
+    source = new McpSource(
+      serverName,
+      {
+        type: "stdio",
+        spawn: {
+          command: server.command,
+          args: server.args,
+          env: {
+            ...server.env,
+            ...filterEnvForBundle(process.env as Record<string, string>, undefined, ref.allowedEnv),
+            ...(ref.env ?? {}),
+            ...platformEnv,
+          },
+          cwd: server.cwd,
+        },
+      },
+      eventSink,
+    );
   } else {
     const internalEnv = ref.protected && opts?.internalEnv ? opts.internalEnv : undefined;
     const result = buildLocalSource(
diff --git a/src/tools/system-tools.ts b/src/tools/system-tools.ts
index 2c5f8db1..2a9eff7e 100644
--- a/src/tools/system-tools.ts
+++ b/src/tools/system-tools.ts
@@ -165,7 +165,7 @@ export async function createSystemTools(
     {
       name: "manage_app",
       description:
-        "Install, uninstall, or configure an app. 'configure' prompts for API keys/credentials securely via the terminal. Requires user approval.",
+        "Install, uninstall, or configure an app. 'configure' prompts for API keys/credentials securely via the terminal. Requires user approval. For uploaded .mcpb bundles, pass 'path' instead of 'name'.",
       inputSchema: {
         type: "object",
         properties: {
@@ -178,12 +178,18 @@ export async function createSystemTools(
             type: "string",
             description: "Bundle name (e.g., @nimblebraininc/ipinfo)",
           },
+          path: {
+            type: "string",
+            description:
+              "Absolute path to a local .mcpb bundle file. Use instead of 'name' for uploaded bundles.",
+          },
         },
-        required: ["action", "name"],
+        required: ["action"],
       },
       handler: async (input): Promise<ToolResult> => {
         const action = String(input.action);
-        const name = String(input.name);
+        const name = input.name ? String(input.name) : undefined;
+        const path = input.path ? String(input.path) : undefined;
         if (!lifecycle || !manageBundleCtx) {
           return {
             content: textContent("Bundle management requires lifecycle context"),
@@ -198,8 +204,14 @@ export async function createSystemTools(
           };
         }
         if (action === "install") {
+          if (!name && !path) {
+            return {
+              content: textContent("Either 'name' or 'path' is required for install"),
+              isError: true,
+            };
+          }
           return await installBundleInWorkspaceViaCtx(
-            name,
+            { name, path },
             wsId,
             lifecycle,
             getRegistry(),
@@ -207,8 +219,15 @@ export async function createSystemTools(
           );
         }
         if (action === "uninstall") {
+          const target = name ?? path;
+          if (!target) {
+            return {
+              content: textContent("Either 'name' or 'path' is required for uninstall"),
+              isError: true,
+            };
+          }
           return await uninstallBundleFromWorkspaceViaCtx(
-            name,
+            target,
             wsId,
             lifecycle,
             getRegistry(),
@@ -216,6 +235,12 @@ export async function createSystemTools(
           );
         }
         if (action === "configure") {
+          if (!name) {
+            return {
+              content: textContent("'name' is required for configure"),
+              isError: true,
+            };
+          }
           return await configureBundle(
             name,
             getRegistry(),
@@ -810,16 +835,18 @@ async function configureBundle(
  * add to workspace.json bundles, seed lifecycle instance.
  */
 async function installBundleInWorkspaceViaCtx(
-  name: string,
+  target: { name?: string; path?: string },
   wsId: string,
   lifecycle: BundleLifecycleManager,
   registry: ToolRegistry,
   ctx: ManageBundleContext,
 ): Promise<ToolResult> {
-  try {
-    const bundleRef = { name } as import("../bundles/types.ts").BundleRef;
+  const label = target.name ?? target.path!;
+  const bundleRef = (
+    target.name ? { name: target.name } : { path: target.path! }
+  ) as import("../bundles/types.ts").BundleRef;
 
-    // Spawn the bundle process with plain server name in workspace registry
+  try {
     const entry = await installBundleInWorkspace(
       wsId,
       bundleRef,
@@ -832,10 +859,9 @@ async function installBundleInWorkspaceViaCtx(
       },
     );
 
-    // Seed lifecycle instance so it can be tracked/queried
     lifecycle.seedInstance(
       entry.serverName,
-      name,
+      label,
       bundleRef,
       entry.meta ?? undefined,
       wsId,
@@ -846,13 +872,15 @@ async function installBundleInWorkspaceViaCtx(
     // a bundle on the user's behalf.
     lifecycle.notifyInstalled(entry.serverName, wsId);
 
-    // Add bundle to workspace.json
     const ws = await ctx.workspaceStore.get(wsId);
     if (ws) {
-      const already = ws.bundles.some((b) => "name" in b && b.name === name);
+      const already = ws.bundles.some((b) => {
+        if (target.name) return "name" in b && b.name === target.name;
+        return "path" in b && b.path === target.path;
+      });
       if (!already) {
         await ctx.workspaceStore.update(wsId, {
-          bundles: [...ws.bundles, { name }],
+          bundles: [...ws.bundles, target.name ? { name: target.name } : { path: target.path! }],
         });
       }
     }
@@ -861,14 +889,14 @@ async function installBundleInWorkspaceViaCtx(
     const count = tools.filter((t) => t.name.startsWith(`${entry.serverName}__`)).length;
     return {
       content: textContent(
-        `Installed ${name} in workspace ${wsId}. ${count} tools now available from ${entry.serverName}.`,
+        `Installed ${label} in workspace ${wsId}. ${count} tools now available from ${entry.serverName}.`,
       ),
       isError: false,
     };
   } catch (err) {
     return {
       content: textContent(
-        `Failed to install ${name} in workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`,
+        `Failed to install ${label} in workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`,
       ),
       isError: true,
     };
diff --git a/web/src/api/client.ts b/web/src/api/client.ts
index 8a34ed31..60ac4272 100644
--- a/web/src/api/client.ts
+++ b/web/src/api/client.ts
@@ -328,6 +328,59 @@ export async function uploadResource(files: File[]): Promise<UploadResourceResul
   return res.json() as Promise<UploadResourceResult>;
 }
 
+// ---------------------------------------------------------------------------
+// Bundle upload
+// ---------------------------------------------------------------------------
+
+export interface BundleUploadResult {
+  path: string;
+  manifest: {
+    name: string;
+    version: string;
+    description: string;
+    display_name: string | null;
+    server_type: string;
+    tools: Array<{ name: string; description?: string }>;
+  };
+}
+
+/**
+ * Upload a .mcpb bundle file for validation. Returns the saved path and
+ * manifest on success. The caller should then trigger install via
+ * `callTool("nb__manage_app", { action: "install", path })`.
+ */
+export async function uploadBundle(file: File): Promise<BundleUploadResult> {
+  const formData = new FormData();
+  formData.append("file", file, file.name);
+
+  const h: Record<string, string> = {};
+  if (authToken && authToken !== "__cookie__") {
+    h.Authorization = `Bearer ${authToken}`;
+  }
+  if (activeWorkspaceId) {
+    h["X-Workspace-Id"] = activeWorkspaceId;
+  }
+
+  const res = await fetchWithRefresh(`${API_BASE}/v1/bundles/upload`, {
+    method: "POST",
+    credentials: "include",
+    headers: h,
+    body: formData,
+  });
+
+  if (res.status === 401) {
+    throw new ApiClientError("unauthorized", "Unauthorized", 401);
+  }
+  if (!res.ok) {
+    const body: ApiError = await res.json().catch(() => ({
+      error: "unknown",
+      message: res.statusText,
+    }));
+    throw new ApiClientError(body.error, body.message, res.status, body.details);
+  }
+  return res.json() as Promise<BundleUploadResult>;
+}
+
 // ---------------------------------------------------------------------------
 // Chat
 // ---------------------------------------------------------------------------
diff --git a/web/src/pages/settings/AboutTab.tsx b/web/src/pages/settings/AboutTab.tsx
index 342677ad..eb536c49 100644
--- a/web/src/pages/settings/AboutTab.tsx
+++ b/web/src/pages/settings/AboutTab.tsx
@@ -1,6 +1,7 @@
-import { useCallback, useEffect, useState } from "react";
+import { Upload } from "lucide-react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { Link } from "react-router-dom";
-import { callTool, getPlatformVersion } from "../../api/client";
+import { callTool, getPlatformVersion, uploadBundle } from "../../api/client";
 import { parseToolResult } from "../../api/tool-result";
 import { Badge } from "../../components/ui/badge";
 import { Button } from "../../components/ui/button";
@@ -61,6 +62,8 @@ export function AboutTab() {
   const [apps, setApps] = useState<AppInfo[]>([]);
   const [loading, setLoading] = useState(true);
   const [bundlesError, setBundlesError] = useState<string | null>(null);
+  const [uploadStatus, setUploadStatus] = useState<string | null>(null);
+  const fileRef = useRef<HTMLInputElement>(null);
 
   const fetchApps = useCallback(async () => {
     try {
@@ -71,11 +74,6 @@ export function AboutTab() {
         setApps(data.apps);
       }
     } catch (err) {
-      // Surface the failure rather than silently degrading to "no bundles
-      // installed" — the empty state would otherwise read as authoritative
-      // ("there are no bundles") when really the call failed and we don't
-      // know. The platform-version section is independent (read from
-      // bootstrap), so the page still renders useful content above.
       setBundlesError(err instanceof Error ? err.message : "Failed to load installed bundles.");
     } finally {
       setLoading(false);
@@ -86,6 +84,39 @@ export function AboutTab() {
     fetchApps();
   }, [fetchApps]);
 
+  const handleUpload = useCallback(
+    async (file: File) => {
+      if (!file.name.endsWith(".mcpb")) {
+        setUploadStatus("File must have .mcpb extension");
+        return;
+      }
+      setUploadStatus("Validating...");
+      try {
+        const result = await uploadBundle(file);
+        setUploadStatus("Installing...");
+        const installResult = await callTool("nb", "manage_app", {
+          action: "install",
+          path: result.path,
+        });
+        if (installResult.isError) {
+          const msg =
+            installResult.content
+              ?.filter((c: { type: string }) => c.type === "text")
+              .map((c: { text: string }) => c.text)
+              .join("") ?? "Install failed";
+          setUploadStatus(msg);
+          return;
+        }
+        setUploadStatus(null);
+        setLoading(true);
+        fetchApps();
+      } catch (err: unknown) {
+        setUploadStatus(err instanceof Error ? err.message : String(err));
+      }
+    },
+    [fetchApps],
+  );
+
   return (
     <SettingsDashboardPage
       title="About"
@@ -100,9 +131,37 @@ export function AboutTab() {
         </dl>
       </Section>
 
-      <Section title="Installed Bundles">
+      <Section
+        title="Installed Bundles"
+        action={
+          <>
+            <input
+              ref={fileRef}
+              type="file"
+              accept=".mcpb"
+              className="hidden"
+              onChange={(e) => {
+                const file = e.target.files?.[0];
+                if (file) handleUpload(file);
+                e.target.value = "";
+              }}
+            />
+            <Button
+              variant="outline"
+              size="sm"
+              onClick={() => fileRef.current?.click()}
+              disabled={uploadStatus === "Validating..." || uploadStatus === "Installing..."}
+            >
+              <Upload className="mr-1 h-3.5 w-3.5" />
+              {uploadStatus === "Validating..." || uploadStatus === "Installing..."
+                ? uploadStatus
+                : "Upload"}
+            </Button>
+          </>
+        }
+      >
         <p className="text-xs text-muted-foreground mb-3">
-          Read-only view. Manage installed bundles in{" "}
+          Read-only view (apart from .mcpb upload). Manage installed bundles in{" "}
           <Link
             to="/settings/workspace/connectors"
             className="text-primary underline-offset-4 hover:underline"
@@ -111,6 +170,18 @@ export function AboutTab() {
           </Link>
           .
         </p>
+        {uploadStatus && uploadStatus !== "Validating..." && uploadStatus !== "Installing..." ? (
+          <InlineError
+            message={uploadStatus}
+            action={
+              <Button variant="outline" size="sm" onClick={() => setUploadStatus(null)}>
+                Dismiss
+              </Button>
+            }
+          />
+        ) : null}
+
+
         {loading ? (
           <p className="text-sm text-muted-foreground">Loading...</p>
         ) : bundlesError ? (

From 986acf6ab400fab91f8f4f45df63b6af4ce9d015 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 00:23:05 -0500
Subject: [PATCH 02/21] style: fix biome formatting in bundle upload files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/api/handlers.ts       | 6 +++++-
 src/api/routes/bundles.ts | 6 ++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index 0dc56b20..2c78cd46 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1393,7 +1393,11 @@ export async function handleBundleUpload(
 
   const fileEntry = formData.get("file") ?? formData.get("bundle");
   if (!fileEntry || typeof fileEntry === "string") {
-    return apiError(400, "bad_request", "No bundle file in request (use the 'file' or 'bundle' field)");
+    return apiError(
+      400,
+      "bad_request",
+      "No bundle file in request (use the 'file' or 'bundle' field)",
+    );
   }
 
   const entry = fileEntry as unknown as {
diff --git a/src/api/routes/bundles.ts b/src/api/routes/bundles.ts
index a04526a1..0b60aadf 100644
--- a/src/api/routes/bundles.ts
+++ b/src/api/routes/bundles.ts
@@ -13,9 +13,7 @@ export function bundleRoutes(ctx: AppContext) {
     .use("*", requireAuth(ctx.authOptions))
     .use("*", requireWorkspace(ctx.workspaceStore))
     .use("*", errorLog(ctx))
-    .post(
-      "/v1/bundles/upload",
-      bodyLimit(1_048_576, { multipart: MAX_BUNDLE_SIZE }),
-      (c) => handleBundleUpload(c.req.raw, ctx.runtime, c.var.workspaceId),
+    .post("/v1/bundles/upload", bodyLimit(1_048_576, { multipart: MAX_BUNDLE_SIZE }), (c) =>
+      handleBundleUpload(c.req.raw, ctx.runtime, c.var.workspaceId),
     );
 }

From e2a6c744bc9a3713e85029ceb3157f41ab8db080 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 00:44:45 -0500
Subject: [PATCH 03/21] fix: remove unused import and sort imports for lint

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/api/app.ts      | 2 +-
 src/api/handlers.ts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/api/app.ts b/src/api/app.ts
index a13a5d74..b8e9d506 100644
--- a/src/api/app.ts
+++ b/src/api/app.ts
@@ -4,6 +4,7 @@ import { corsMiddleware } from "./middleware/cors.ts";
 import { securityHeaders } from "./middleware/security-headers.ts";
 import { authRoutes } from "./routes/auth.ts";
 import { bootstrapRoutes } from "./routes/bootstrap.ts";
+import { bundleRoutes } from "./routes/bundles.ts";
 import { chatRoutes } from "./routes/chat.ts";
 import { conversationEventRoutes } from "./routes/conversation-events.ts";
 import { eventRoutes } from "./routes/events.ts";
@@ -11,7 +12,6 @@ import { healthRoutes } from "./routes/health.ts";
 import { mcpRoutes } from "./routes/mcp.ts";
 import { mcpAuthRoutes } from "./routes/mcp-auth.ts";
 import { proxyRoutes } from "./routes/proxy.ts";
-import { bundleRoutes } from "./routes/bundles.ts";
 import { resourceRoutes } from "./routes/resources.ts";
 import { toolRoutes } from "./routes/tools.ts";
 import { wellKnownRoutes } from "./routes/well-known.ts";
diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index 2c78cd46..d4f62c05 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1420,7 +1420,7 @@ export async function handleBundleUpload(
   }
 
   // Save to workspace bundles directory
-  const { existsSync, mkdirSync, writeFileSync } = await import("node:fs");
+  const { mkdirSync, writeFileSync } = await import("node:fs");
   const { join: joinPath } = await import("node:path");
 
   const bundlesDir = joinPath(runtime.getWorkspaceScopedDir(workspaceId), "bundles");

From af5568e33affdb5a3bed5bfead0a6f32ebdcd9ec Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 22:29:05 -0500
Subject: [PATCH 04/21] test: add failing tests for PR #170 .mcpb upload fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Drives the next four commits via TDD red→green. Each test asserts
post-fix behavior and currently fails against the buggy code:

- Fix 1 (path traversal in handleBundleUpload)
- Fix 3a (uninstall server name resolution by .mcpb path)
- Fix 3b (workspace.json filter handles {path} entries)
- Fix 4 (.mcpb branch resolves workspace credentials, requires wsId)
- Fix 5 (.mcpb spawn env includes MPAK_WORKSPACE / UPJACK_ROOT / internalEnv)
- Fix 6 (install returns manifest-derived sourceName + dataDir)

13 bug-tests fail; 1 control test (named-bundle removal) passes —
confirming the filter test isolates the .mcpb regression.

Integration tests (Fix 4, 5, 6) use bun:test mock.module to stub
McpSource, getMpak, workspace-credentials so startBundleSource and
installBundleInWorkspace can be exercised in isolation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 test/unit/mcpb-upload.test.ts | 498 ++++++++++++++++++++++++++++++++++
 1 file changed, 498 insertions(+)
 create mode 100644 test/unit/mcpb-upload.test.ts

diff --git a/test/unit/mcpb-upload.test.ts b/test/unit/mcpb-upload.test.ts
new file mode 100644
index 00000000..0b4d9940
--- /dev/null
+++ b/test/unit/mcpb-upload.test.ts
@@ -0,0 +1,498 @@
+/**
+ * Tests asserting CORRECT behavior for .mcpb bundle upload fixes.
+ * These FAIL against the current (buggy) code and PASS after fixes.
+ *
+ * Covers PR #170 review issues:
+ * 1. Path traversal in handleBundleUpload filename
+ * 3a. Uninstall by path: server name resolution (path-derived ≠ manifest)
+ * 3b. Uninstall by path: workspace.json filter must handle {path} entries
+ * 4. .mcpb branch must call resolveUserConfig (workspace credentials)
+ * 5. Missing MPAK_WORKSPACE / UPJACK_ROOT in .mcpb env
+ * 6. Server name collision on install (path-derived ≠ manifest-derived)
+ *
+ * Fix 2 (SDK dep blocker) is a PR comment, not a code change — no test.
+ */
+
+import { describe, expect, it, mock, beforeEach, afterEach } from "bun:test";
+import { join } from "node:path";
+import { mkdirSync, writeFileSync, rmSync } from "node:fs";
+import { deriveServerName } from "../../src/bundles/paths.ts";
+
+// ---------------------------------------------------------------------------
+// Fix 1: Path traversal — uploaded filename must strip directory components
+//
+// handleBundleUpload does: join(bundlesDir, sanitizeFilename(filename))
+// sanitizeFilename only strips control chars / quotes, so "../../etc/foo.mcpb"
+// passes through unchanged → written path escapes bundlesDir.
+//
+// Fix introduces an exported helper `safeBundleFilename` that the handler
+// uses to derive on-disk filename. Helper applies path.basename so all
+// directory components are stripped.
+// ---------------------------------------------------------------------------
+
+describe("Fix 1: path traversal in handleBundleUpload", () => {
+	it("safeBundleFilename strips traversal components", async () => {
+		const handlersModule = await import("../../src/api/handlers.ts");
+		const safeBundleFilename = (
+			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
+		).safeBundleFilename;
+
+		// CORRECT: helper must be exported by handlers.ts
+		expect(typeof safeBundleFilename).toBe("function");
+		// CORRECT: traversal components stripped to plain filename
+		expect(safeBundleFilename!("../../etc/cron.daily/evil.mcpb")).toBe(
+			"evil.mcpb",
+		);
+	});
+
+	it("safeBundleFilename strips absolute path components", async () => {
+		const handlersModule = await import("../../src/api/handlers.ts");
+		const safeBundleFilename = (
+			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
+		).safeBundleFilename;
+
+		expect(typeof safeBundleFilename).toBe("function");
+		expect(safeBundleFilename!("/tmp/secrets/payload.mcpb")).toBe(
+			"payload.mcpb",
+		);
+	});
+
+	it("joined path with safeBundleFilename stays inside bundlesDir", async () => {
+		const handlersModule = await import("../../src/api/handlers.ts");
+		const safeBundleFilename = (
+			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
+		).safeBundleFilename;
+
+		expect(typeof safeBundleFilename).toBe("function");
+		const bundlesDir = "/home/.nimblebrain/workspaces/ws_dev/bundles";
+		const result = join(bundlesDir, safeBundleFilename!("../../evil.mcpb"));
+		expect(result.startsWith(bundlesDir)).toBe(true);
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 3: Uninstall workspace.json filter must handle {path} entries
+//
+// uninstallBundleFromWorkspaceViaCtx filters workspace.json bundles with:
+//   ws.bundles.filter((b) => !("name" in b && b.name === name))
+// This only removes {name} entries. {path} entries are never matched because
+// the target could be a path string, not a name string.
+// ---------------------------------------------------------------------------
+
+// ---------------------------------------------------------------------------
+// Fix 3a: uninstall by path — server name must come from manifest, not path
+//
+// uninstallBundleFromWorkspaceViaCtx calls deriveServerName(name) where `name`
+// can be a `.mcpb` path string. For a path like "/uploads/echo.mcpb",
+// deriveServerName("/uploads/echo.mcpb") = "echo-mcpb". But the source was
+// registered under deriveServerName(manifest.name) = "echo".
+// → registry lookup misses, uninstall reports "no bundle found".
+// ---------------------------------------------------------------------------
+
+describe("Fix 3a: uninstall by .mcpb path resolves to manifest-derived name", () => {
+	it("path-as-name derived must equal manifest-derived name", () => {
+		// Current uninstall: deriveServerName(name) where name = the path string
+		const uninstallDerived = deriveServerName("/uploads/echo.mcpb");
+		// Actual registered: deriveServerName(manifest.name) = "echo"
+		const registeredAs = deriveServerName("echo");
+		// CORRECT: uninstall must resolve to the manifest-derived registered name
+		expect(uninstallDerived).toBe(registeredAs);
+	});
+});
+
+describe("Fix 3b: uninstall workspace.json filter handles {path} entries", () => {
+	/**
+	 * Replicates the CURRENT (buggy) filter from uninstallBundleFromWorkspaceViaCtx.
+	 * It only matches by name — path entries slip through.
+	 */
+	function currentFilter(
+		bundles: Array<{ name?: string; path?: string }>,
+		target: string,
+	): Array<{ name?: string; path?: string }> {
+		return bundles.filter((b) => !("name" in b && b.name === target));
+	}
+
+	it("removing a path-based bundle must actually remove the {path} entry", () => {
+		const bundles = [
+			{ name: "@acme/hello" },
+			{ path: "/uploads/custom.mcpb" },
+			{ name: "@acme/world" },
+		];
+
+		// Uninstalling the path-based bundle by its path
+		const result = currentFilter(bundles, "/uploads/custom.mcpb");
+
+		// CORRECT: should have 2 entries (path entry removed)
+		expect(result).toHaveLength(2);
+		expect(
+			result.some((b) => "path" in b && b.path === "/uploads/custom.mcpb"),
+		).toBe(false);
+	});
+
+	it("removing a named bundle still works", () => {
+		const bundles = [
+			{ name: "@acme/hello" },
+			{ path: "/uploads/custom.mcpb" },
+			{ name: "@acme/world" },
+		];
+
+		// Uninstalling a named bundle — this already works in current code
+		const result = currentFilter(bundles, "@acme/hello");
+
+		expect(result).toHaveLength(2);
+		expect(
+			result.some((b) => "name" in b && b.name === "@acme/hello"),
+		).toBe(false);
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 5: .mcpb spawn env must include MPAK_WORKSPACE and UPJACK_ROOT
+//
+// The named-bundle branch in startBundleSource sets:
+//   MPAK_WORKSPACE: bundleDataDir, UPJACK_ROOT: bundleDataDir
+// The .mcpb branch omits these — bundles that depend on them break silently.
+//
+// Uses mock.module to intercept McpSource construction and capture spawn env.
+// ---------------------------------------------------------------------------
+
+let capturedSpawnEnv: Record<string, string> | undefined;
+let capturedResolveUserConfigOpts:
+	| { bundleName: string; userConfigSchema: unknown; wsId: string }
+	| undefined;
+let capturedPrepareServerOpts: { workspaceDir: string; userConfig?: unknown } | undefined;
+
+mock.module("../../src/config/workspace-credentials.ts", () => ({
+	resolveUserConfig: async (opts: {
+		bundleName: string;
+		userConfigSchema: unknown;
+		wsId: string;
+		workDir: string;
+	}) => {
+		capturedResolveUserConfigOpts = {
+			bundleName: opts.bundleName,
+			userConfigSchema: opts.userConfigSchema,
+			wsId: opts.wsId,
+		};
+		return { api_key: "from-workspace-creds" };
+	},
+	friendlyMpakConfigError: (err: unknown) => err,
+	getWorkspaceCredentials: async () => ({}),
+	clearAllWorkspaceCredentials: async () => {},
+}));
+
+mock.module("../../src/tools/mcp-source.ts", () => ({
+	McpSource: class MockMcpSource {
+		name: string;
+		constructor(name: string, config: { type: string; spawn?: { env?: Record<string, string> } }) {
+			this.name = name;
+			if (config?.type === "stdio" && config.spawn?.env) {
+				capturedSpawnEnv = { ...config.spawn.env };
+			}
+		}
+		async start() {}
+		async stop() {}
+		async tools() {
+			return [];
+		}
+		async execute() {
+			return { content: [], isError: true as const };
+		}
+	},
+}));
+
+mock.module("../../src/bundles/mpak.ts", () => ({
+	getMpak: () => ({
+		prepareServer: async (
+			_ref: unknown,
+			opts: { workspaceDir: string; userConfig?: unknown },
+		) => {
+			capturedPrepareServerOpts = opts;
+			return {
+				command: "node",
+				args: ["index.js"],
+				env: { MPAK_SDK_VAR: "from-sdk" },
+				cwd: "/tmp/nb-test-mcpb-env",
+				name: "echo",
+			};
+		},
+		bundleCache: {
+			getBundleManifest: () => null,
+		},
+	}),
+}));
+
+const tmpCwd = "/tmp/nb-test-mcpb-env";
+const testManifest = {
+	name: "echo",
+	version: "1.0.0",
+	description: "Test bundle",
+	user_config: {
+		api_key: { type: "string", title: "API Key", required: true },
+	},
+	server: {
+		type: "node",
+		mcp_config: {
+			command: "node",
+			args: ["index.js"],
+			env: { ECHO_API_KEY: "${user_config.api_key}" },
+		},
+	},
+};
+
+async function runStartBundleSourceForMcpb() {
+	const { startBundleSource } = await import("../../src/bundles/startup.ts");
+	const { ToolRegistry } = await import("../../src/tools/registry.ts");
+	const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
+
+	const registry = new ToolRegistry();
+	const sink = new NoopEventSink();
+	const ref = { path: "/test/echo.mcpb" };
+
+	await startBundleSource(ref, registry, sink, undefined, {
+		wsId: "ws_test",
+		workDir: "/tmp/nb-workdir",
+		dataDir: "/tmp/nb-workdir/data/echo",
+	});
+}
+
+describe("Fix 5: .mcpb spawn env includes MPAK_WORKSPACE and UPJACK_ROOT", () => {
+	beforeEach(() => {
+		capturedSpawnEnv = undefined;
+		capturedResolveUserConfigOpts = undefined;
+		capturedPrepareServerOpts = undefined;
+		mkdirSync(tmpCwd, { recursive: true });
+		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
+	});
+
+	afterEach(() => {
+		rmSync(tmpCwd, { recursive: true, force: true });
+	});
+
+	it("spawn env must include MPAK_WORKSPACE and UPJACK_ROOT", async () => {
+		await runStartBundleSourceForMcpb();
+
+		expect(capturedSpawnEnv).toBeDefined();
+		// CORRECT: both env vars must be set to the data dir
+		expect(capturedSpawnEnv!.MPAK_WORKSPACE).toBe("/tmp/nb-workdir/data/echo");
+		expect(capturedSpawnEnv!.UPJACK_ROOT).toBe("/tmp/nb-workdir/data/echo");
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 4: .mcpb branch must call resolveUserConfig and pass userConfig
+//
+// The named-bundle branch resolves workspace credentials via resolveUserConfig
+// before calling prepareServer. The .mcpb branch skips this entirely → bundles
+// declaring user_config never see workspace-stored credentials, and the SDK's
+// missing-required check fires even when creds are configured.
+// ---------------------------------------------------------------------------
+
+describe("Fix 4: .mcpb branch resolves workspace credentials", () => {
+	beforeEach(() => {
+		capturedSpawnEnv = undefined;
+		capturedResolveUserConfigOpts = undefined;
+		capturedPrepareServerOpts = undefined;
+		mkdirSync(tmpCwd, { recursive: true });
+		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
+	});
+
+	afterEach(() => {
+		rmSync(tmpCwd, { recursive: true, force: true });
+	});
+
+	it("resolveUserConfig must be called with manifest name and user_config schema", async () => {
+		await runStartBundleSourceForMcpb();
+
+		expect(capturedResolveUserConfigOpts).toBeDefined();
+		// Must use manifest-derived bundle name, not the file path
+		expect(capturedResolveUserConfigOpts!.bundleName).toBe("echo");
+		expect(capturedResolveUserConfigOpts!.wsId).toBe("ws_test");
+		// Must thread the manifest's user_config schema for credential resolution
+		expect(capturedResolveUserConfigOpts!.userConfigSchema).toEqual(
+			testManifest.user_config,
+		);
+	});
+
+	it("resolved userConfig must be passed to prepareServer", async () => {
+		await runStartBundleSourceForMcpb();
+
+		expect(capturedPrepareServerOpts).toBeDefined();
+		// CORRECT: resolved creds must reach the SDK's prepareServer
+		expect(capturedPrepareServerOpts!.userConfig).toEqual({
+			api_key: "from-workspace-creds",
+		});
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 4 (continued): .mcpb branch must require opts.wsId
+//
+// The named-bundle branch hard-errors when wsId is missing (workspace-scoped
+// credential resolution requires it). The .mcpb branch must do the same —
+// silently defaulting would pool credentials across tenants.
+// ---------------------------------------------------------------------------
+
+describe("Fix 4 (continued): .mcpb branch requires opts.wsId", () => {
+	beforeEach(() => {
+		capturedSpawnEnv = undefined;
+		mkdirSync(tmpCwd, { recursive: true });
+		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
+	});
+
+	afterEach(() => {
+		rmSync(tmpCwd, { recursive: true, force: true });
+	});
+
+	it("startBundleSource for .mcpb throws when wsId missing", async () => {
+		const { startBundleSource } = await import("../../src/bundles/startup.ts");
+		const { ToolRegistry } = await import("../../src/tools/registry.ts");
+		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
+
+		const registry = new ToolRegistry();
+		const sink = new NoopEventSink();
+		const ref = { path: "/test/echo.mcpb" };
+
+		// CORRECT: must throw because workspace-scoped credentials cannot resolve
+		// without wsId. Currently passes silently — credentials never resolve.
+		await expect(
+			startBundleSource(ref, registry, sink, undefined, {
+				workDir: "/tmp/nb-workdir",
+			}),
+		).rejects.toThrow(/workspace/i);
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 5 (continued): protected .mcpb spawn env includes internalEnv
+//
+// Named/local branches inject NB_INTERNAL_TOKEN + NB_HOST_URL into spawn env
+// when ref.protected is true and opts.internalEnv is provided. The .mcpb
+// branch never threads internalEnv → protected bundles cannot reach the host.
+// ---------------------------------------------------------------------------
+
+describe("Fix 5 (continued): protected .mcpb gets internalEnv", () => {
+	beforeEach(() => {
+		capturedSpawnEnv = undefined;
+		mkdirSync(tmpCwd, { recursive: true });
+		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
+	});
+
+	afterEach(() => {
+		rmSync(tmpCwd, { recursive: true, force: true });
+	});
+
+	it("spawn env must include NB_INTERNAL_TOKEN and NB_HOST_URL when ref.protected", async () => {
+		const { startBundleSource } = await import("../../src/bundles/startup.ts");
+		const { ToolRegistry } = await import("../../src/tools/registry.ts");
+		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
+
+		const registry = new ToolRegistry();
+		const sink = new NoopEventSink();
+		const ref = { path: "/test/echo.mcpb", protected: true };
+
+		await startBundleSource(ref, registry, sink, undefined, {
+			wsId: "ws_test",
+			workDir: "/tmp/nb-workdir",
+			dataDir: "/tmp/nb-workdir/data/echo",
+			internalEnv: {
+				NB_INTERNAL_TOKEN: "secret-token",
+				NB_HOST_URL: "http://localhost:27247",
+			},
+		});
+
+		expect(capturedSpawnEnv).toBeDefined();
+		// CORRECT: protected .mcpb must receive host-comm credentials
+		expect(capturedSpawnEnv!.NB_INTERNAL_TOKEN).toBe("secret-token");
+		expect(capturedSpawnEnv!.NB_HOST_URL).toBe("http://localhost:27247");
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 6: Server name collision — path-derived ≠ manifest-derived
+//
+// workspace-ops.ts serverNameFromRef uses deriveServerName(ref.path) for path
+// refs. But startBundleSource registers .mcpb bundles under
+// deriveServerName(manifest.name). These differ because the path includes
+// the ".mcpb" extension: deriveServerName("echo.mcpb") → "echo-mcpb",
+// but deriveServerName("echo") → "echo".
+//
+// installBundleInWorkspace pre-computes the server name from the path for its
+// duplicate check, but the actual registered name comes from the manifest.
+// The duplicate check uses the wrong name → silent collision.
+// ---------------------------------------------------------------------------
+
+describe("Fix 6: .mcpb server name must come from manifest, not file path", () => {
+	it("path-derived name must equal manifest-derived name for .mcpb", () => {
+		// serverNameFromRef({path: "/uploads/echo.mcpb"}) calls
+		// deriveServerName("/uploads/echo.mcpb")
+		const pathDerived = deriveServerName("/uploads/echo.mcpb");
+
+		// startBundleSource registers under deriveServerName(manifest.name)
+		// where manifest.name = "echo"
+		const manifestDerived = deriveServerName("echo");
+
+		// CORRECT: these must be equal so the duplicate check works
+		// Currently: pathDerived = "echo-mcpb", manifestDerived = "echo"
+		expect(pathDerived).toBe(manifestDerived);
+	});
+
+	it("scoped name path-derived must equal manifest-derived", () => {
+		const pathDerived = deriveServerName("/uploads/@acme/my-tool.mcpb");
+		const manifestDerived = deriveServerName("my-tool");
+
+		// Both should resolve to "my-tool"
+		expect(pathDerived).toBe(manifestDerived);
+	});
+});
+
+// ---------------------------------------------------------------------------
+// Fix 6 (continued): installBundleInWorkspace returns dataDir from manifest
+//
+// installBundleInWorkspace currently computes dataDir from
+// `bundleNameFromRef(ref) = ref.path` for path refs. For .mcpb,
+// `deriveBundleDataDir("/uploads/echo.mcpb")` produces a non-portable
+// path-derived dir. After fix, dataDir comes from manifest name.
+// ---------------------------------------------------------------------------
+
+describe("Fix 6 (continued): install returns manifest-derived dataDir", () => {
+	beforeEach(() => {
+		capturedSpawnEnv = undefined;
+		mkdirSync(tmpCwd, { recursive: true });
+		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
+	});
+
+	afterEach(() => {
+		rmSync(tmpCwd, { recursive: true, force: true });
+	});
+
+	it("install with .mcpb path returns dataDir derived from manifest, not path", async () => {
+		const { installBundleInWorkspace } = await import(
+			"../../src/bundles/workspace-ops.ts"
+		);
+		const { ToolRegistry } = await import("../../src/tools/registry.ts");
+		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
+
+		const registry = new ToolRegistry();
+		const sink = new NoopEventSink();
+
+		const result = await installBundleInWorkspace(
+			"ws_test",
+			{ path: "/uploads/echo.mcpb" },
+			registry,
+			sink,
+			undefined,
+			{ workDir: "/tmp/nb-workdir" },
+		);
+
+		// CORRECT: serverName from manifest
+		expect(result.serverName).toBe("echo");
+		// CORRECT: dataDir from manifest name, portable across re-uploads
+		expect(result.dataDir).toBe(
+			join("/tmp/nb-workdir/workspaces/ws_test/data", "echo"),
+		);
+		// Negative: must NOT contain path components
+		expect(result.dataDir).not.toContain("uploads");
+		expect(result.dataDir).not.toContain("mcpb");
+	});
+});

From a6721b51db4850501ad4267f0513195c89987c55 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 22:29:48 -0500
Subject: [PATCH 05/21] fix: prevent path traversal in .mcpb bundle upload
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

handleBundleUpload joined the user-supplied filename onto the workspace
bundles dir after only stripping control chars / quotes via
sanitizeFilename. A filename like "../../etc/cron.daily/evil.mcpb" passed
through unchanged and the writeFileSync target escaped bundlesDir,
landing wherever the user could traverse to.

Fix: introduce safeBundleFilename which applies path.basename, stripping
all directory components. handleBundleUpload now uses this helper for
the on-disk path. Helper is exported so the test pins the contract.

sanitizeFilename is unchanged — it remains the right tool for the
Content-Disposition path (file download), where we want to preserve
display names but neutralize header-injection chars.

Closes PR #170 review comment 1.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/api/handlers.ts | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index d4f62c05..3e8cd49d 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1,5 +1,5 @@
 import { readFileSync } from "node:fs";
-import { join, resolve } from "node:path";
+import { basename, join, resolve } from "node:path";
 import { CallbackEventSink } from "../adapters/callback-events.ts";
 import { log } from "../cli/log.ts";
 import { isToolEnabled, isToolVisibleToRole, type ResolvedFeatures } from "../config/features.ts";
@@ -1040,6 +1040,20 @@ export function sanitizeFilename(name: string): string {
   return name.replace(/["\r\n\x00-\x1f]/g, "_");
 }
 
+/**
+ * Resolve the safe on-disk filename for an uploaded `.mcpb` bundle.
+ *
+ * Strips every directory component via `path.basename`, so filenames like
+ * `../../etc/cron.daily/evil.mcpb` collapse to `evil.mcpb` and cannot escape
+ * the workspace bundles dir when joined onto it. `sanitizeFilename` only
+ * neutralizes Content-Disposition-breaking chars and is insufficient on its
+ * own — it would let `..` segments through and the upload would write
+ * outside `bundlesDir`. Exported so tests pin the contract.
+ */
+export function safeBundleFilename(filename: string): string {
+  return basename(filename);
+}
+
 /**
  * Regex for valid file IDs.
  *  - New scheme: `fl_<24 hex chars>` (randomBytes(12).hex).
@@ -1426,7 +1440,7 @@ export async function handleBundleUpload(
   const bundlesDir = joinPath(runtime.getWorkspaceScopedDir(workspaceId), "bundles");
   mkdirSync(bundlesDir, { recursive: true });
 
-  const safeName = sanitizeFilename(filename);
+  const safeName = safeBundleFilename(filename);
   const bundlePath = joinPath(bundlesDir, safeName);
   writeFileSync(bundlePath, data, { mode: 0o600 });
 

From 593fd3f9ec01e73b92fafcfbb1cd88397c8f8964 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 22:31:20 -0500
Subject: [PATCH 06/21] fix: thread credentials and env vars through .mcpb
 startup branch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The .mcpb branch in startBundleSource diverged from the named-bundle
branch in three ways that broke uploaded bundles silently:

1. Skipped resolveUserConfig — workspace-stored credentials (API keys
   declared in manifest.user_config) never reached the subprocess. The
   SDK's MpakConfigError surfaced to the user even when they had run
   `nb config set -w <wsId>` correctly.

2. Derived serverName from server.name (a path-influenced string from
   the SDK) instead of peeking the manifest. dataDir defaulted to
   nbWorkDir root, so Upjack apps wrote outside their per-bundle dir.

3. Omitted MPAK_WORKSPACE / UPJACK_ROOT / internalEnv from the spawn
   env. Upjack data writes landed in the wrong dir; protected bundles
   couldn't reach the host.

Fix: rewrite the branch to mirror the named-bundle path one-for-one,
peeking the manifest from the .mcpb archive via validateMcpb instead
of reading from the mpak cache. Now requires opts.wsId (matches named
branch) so cross-tenant credential pooling is impossible.

Note: validateMcpb requires mpak-sdk@>=0.7.0 (mpak#94). PR #170 stays
blocked on that release; once the dep is bumped, this branch works
end-to-end. Tests mock @nimblebrain/mpak-sdk so they pass against the
currently-installed SDK.

Closes PR #170 review comments 4 and 5.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/bundles/startup.ts        | 97 +++++++++++++++++++++++++++--------
 test/unit/mcpb-upload.test.ts | 10 ++++
 2 files changed, 85 insertions(+), 22 deletions(-)

diff --git a/src/bundles/startup.ts b/src/bundles/startup.ts
index 02ff65c3..8807121c 100644
--- a/src/bundles/startup.ts
+++ b/src/bundles/startup.ts
@@ -486,35 +486,93 @@ export async function startBundleSource(
       eventSink,
     );
   } else if ("path" in ref && ref.path.endsWith(".mcpb")) {
-    // .mcpb archive — use the SDK to extract and prepare
+    // .mcpb archive — peek manifest, resolve workspace credentials, then prepare.
+    //
+    // The original implementation diverged from the named-bundle branch in
+    // three ways: it skipped resolveUserConfig (so workspace-stored API keys
+    // never reached the subprocess), it computed serverName from the file
+    // path (so duplicate checks and lifecycle lookups missed the
+    // manifest-derived name the bundle was actually registered under), and
+    // it omitted MPAK_WORKSPACE / UPJACK_ROOT / internalEnv from the spawn
+    // env (so Upjack data writes landed in the wrong dir and protected
+    // bundles couldn't reach the host). This branch now mirrors the named
+    // path one-for-one, with the manifest peeked from the .mcpb archive
+    // instead of read from the mpak cache.
+    if (!opts?.wsId) {
+      throw new Error(
+        `Cannot start ${ref.path}: a workspace ID is required (.mcpb credentials are workspace-scoped).`,
+      );
+    }
+
+    const nbWorkDir = opts.workDir ?? process.env.NB_WORK_DIR ?? join(homedir(), ".nimblebrain");
     const mpakHome = process.env.MPAK_HOME ?? join(homedir(), ".mpak");
     const mpak = getMpak(mpakHome);
-    const nbWorkDir = opts?.workDir ?? process.env.NB_WORK_DIR ?? join(homedir(), ".nimblebrain");
 
-    const server = await mpak.prepareServer(
-      { local: ref.path },
-      { workspaceDir: opts?.dataDir ?? nbWorkDir },
-    );
+    // Peek manifest BEFORE prepareServer so we can resolve user_config and
+    // derive serverName from manifest content. validateMcpb extracts to its
+    // own temp dir and cleans up; prepareServer extracts again into the mpak
+    // cache. Double extraction is acceptable — archives are bounded in size
+    // and this only runs at startup.
+    const { validateMcpb } = await import("@nimblebrain/mpak-sdk");
+    const validation = await validateMcpb(ref.path);
+    if (!validation.valid || !validation.manifest) {
+      throw new Error(
+        `Invalid .mcpb bundle at ${ref.path}: ${validation.errors?.join(", ") ?? "manifest validation failed"}`,
+      );
+    }
+    const peekedManifest = validation.manifest as BundleManifest;
+    manifest = peekedManifest;
+    meta = extractBundleMeta(peekedManifest as unknown as Record<string, unknown>);
 
-    const serverName = deriveServerName(server.name);
+    const serverName = deriveServerName(peekedManifest.name);
     validateServerName(serverName);
 
-    // Read manifest from the extracted cache dir
-    const manifestPath = join(server.cwd, "manifest.json");
-    const rawManifest = JSON.parse(readFileSync(manifestPath, "utf-8"));
-    const manifestResult = validateManifest(rawManifest);
-    if (manifestResult.valid && manifestResult.manifest) {
-      manifest = manifestResult.manifest;
-      meta = extractBundleMeta(rawManifest);
+    // Data dir derives from manifest name + wsId — matches the named-bundle
+    // convention so re-uploads of the same bundle land in the same dir
+    // across restarts. opts.dataDir override only used by tests.
+    const bundleDataDir =
+      opts.dataDir ??
+      resolveBundleDataDir(join(nbWorkDir, "workspaces", opts.wsId), peekedManifest.name);
+
+    const userConfig = await resolveUserConfig({
+      bundleName: peekedManifest.name,
+      userConfigSchema: peekedManifest.user_config,
+      wsId: opts.wsId,
+      workDir: nbWorkDir,
+    });
+
+    let server: Awaited<ReturnType<typeof mpak.prepareServer>>;
+    try {
+      server = await mpak.prepareServer(
+        { local: ref.path },
+        { workspaceDir: bundleDataDir, userConfig },
+      );
+    } catch (err) {
+      throw friendlyMpakConfigError(err, opts.wsId);
     }
 
+    const internalEnv = ref.protected && opts?.internalEnv ? opts.internalEnv : undefined;
+
     const platformEnv = buildPlatformEnv({
-      workspaceId: opts?.wsId,
+      workspaceId: opts.wsId,
       serverName,
-      manifestMeta: (manifest?._meta ?? undefined) as Record<string, unknown> | undefined,
+      manifestMeta: peekedManifest._meta as Record<string, unknown> | undefined,
       publicOrigin: resolvePublicOrigin(),
     });
 
+    const spawnEnv: Record<string, string> = {
+      ...server.env,
+      ...filterEnvForBundle(process.env as Record<string, string>, undefined, ref.allowedEnv),
+      ...(ref.env ?? {}),
+      MPAK_WORKSPACE: bundleDataDir,
+      UPJACK_ROOT: bundleDataDir,
+      ...platformEnv,
+    };
+    if (internalEnv) {
+      spawnEnv.NB_INTERNAL_TOKEN = internalEnv.NB_INTERNAL_TOKEN;
+      spawnEnv.NB_HOST_URL = internalEnv.NB_HOST_URL;
+    }
+
     source = new McpSource(
       serverName,
       {
@@ -522,12 +580,7 @@ export async function startBundleSource(
         spawn: {
           command: server.command,
           args: server.args,
-          env: {
-            ...server.env,
-            ...filterEnvForBundle(process.env as Record<string, string>, undefined, ref.allowedEnv),
-            ...(ref.env ?? {}),
-            ...platformEnv,
-          },
+          env: spawnEnv,
           cwd: server.cwd,
         },
       },
diff --git a/test/unit/mcpb-upload.test.ts b/test/unit/mcpb-upload.test.ts
index 0b4d9940..6140823a 100644
--- a/test/unit/mcpb-upload.test.ts
+++ b/test/unit/mcpb-upload.test.ts
@@ -162,6 +162,16 @@ let capturedResolveUserConfigOpts:
 	| undefined;
 let capturedPrepareServerOpts: { workspaceDir: string; userConfig?: unknown } | undefined;
 
+// validateMcpb ships in mpak-sdk@>=0.7.0 (mpak#94). Until that version is
+// published + dep-bumped, mock it here so the .mcpb branch is testable.
+mock.module("@nimblebrain/mpak-sdk", () => ({
+	validateMcpb: async (_path: string) => ({
+		valid: true,
+		manifest: testManifest,
+		errors: [],
+	}),
+}));
+
 mock.module("../../src/config/workspace-credentials.ts", () => ({
 	resolveUserConfig: async (opts: {
 		bundleName: string;

From 6aa45b3038d4fec4154a1d71bccfd3413f72823d Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 22:33:31 -0500
Subject: [PATCH 07/21] fix: derive .mcpb server name and dataDir from
 manifest, not file path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

installBundleInWorkspace was pre-computing serverName + dataDir from
the BundleRef before calling startBundleSource. For .mcpb refs that's
wrong: the path-derived name ("echo-mcpb" for "/uploads/echo.mcpb")
doesn't match the manifest-derived name the bundle actually registers
under ("echo"), so:

- The duplicate-source pre-flight check looked up the wrong name. When
  a re-upload landed, the check passed silently and addSource later
  threw the cryptic "already registered" error from the registry.
- dataDir embedded path components ("data/-uploads/echo.mcpb"), so
  re-uploads from a different upload path or a different workspace
  layout landed in different dirs across restarts.

Fix: introduce isMcpbRef helper. For .mcpb refs, skip the pre-flight
name + dataDir derivation and let startBundleSource own registration
(the registry's own duplicate check is the safety net). After start,
derive dataDir from result.sourceName (the manifest-derived name) so
the per-bundle dir is stable across re-uploads.

The test that originally asserted `deriveServerName(path) ===
deriveServerName(name)` was rewritten — that's an impossible contract,
since the path always carries the .mcpb suffix. The new test exercises
the real install path and asserts the registered name matches the
manifest, not the path.

Closes PR #170 review comment 6.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/bundles/workspace-ops.ts  | 44 +++++++++++++++++++++-----
 test/unit/mcpb-upload.test.ts | 58 ++++++++++++++++++++++++-----------
 2 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/src/bundles/workspace-ops.ts b/src/bundles/workspace-ops.ts
index 6bdeb4b4..ebab5ba1 100644
--- a/src/bundles/workspace-ops.ts
+++ b/src/bundles/workspace-ops.ts
@@ -21,6 +21,17 @@ export interface ProcessInventoryEntry {
   meta?: import("./types.ts").LocalBundleMeta | null;
 }
 
+/**
+ * True if this ref is a `.mcpb` archive — the only ref variant whose canonical
+ * server name and data-dir cannot be known until the manifest is peeked at
+ * startup. All other variants (name/url, plain local-dir paths) derive a stable
+ * name from `serverNameFromRef` directly. Callers must defer .mcpb registration
+ * to `startBundleSource`, which peeks the manifest and uses `result.sourceName`.
+ */
+function isMcpbRef(ref: BundleRef): ref is { path: string } & BundleRef {
+  return "path" in ref && ref.path.endsWith(".mcpb");
+}
+
 /**
  * Install a bundle in a specific workspace (hot — no restart required).
  *
@@ -41,26 +52,43 @@ export async function installBundleInWorkspace(
   },
 ): Promise<ProcessInventoryEntry> {
   const workDir = opts?.workDir ?? process.env.NB_WORK_DIR ?? "";
-  const serverName = serverNameFromRef(bundleRef);
-  const bundleName = bundleNameFromRef(bundleRef);
   const wsPath = join(workDir, "workspaces", wsId);
-  const dataDir = resolveBundleDataDir(wsPath, bundleName);
+  const isMcpb = isMcpbRef(bundleRef);
 
-  // Check for existing registration
-  if (registry.hasSource(serverName)) {
-    throw new Error(`Bundle "${serverName}" is already running in workspace "${wsId}"`);
+  // Pre-flight duplicate check + dataDir derivation only run for refs whose
+  // canonical name is knowable from the ref string itself. For .mcpb, the
+  // manifest-derived name lives inside the archive — startBundleSource peeks
+  // it. Pre-computing here would use the path-derived name ("echo-mcpb" for
+  // "/uploads/echo.mcpb") which won't match the actual registered name
+  // ("echo"). Skip and let startBundleSource own the registration; if the
+  // name is already registered, registry.addSource throws.
+  let preflightDataDir: string | undefined;
+  if (!isMcpb) {
+    const serverName = serverNameFromRef(bundleRef);
+    if (registry.hasSource(serverName)) {
+      throw new Error(`Bundle "${serverName}" is already running in workspace "${wsId}"`);
+    }
+    preflightDataDir = resolveBundleDataDir(wsPath, bundleNameFromRef(bundleRef));
   }
 
   const result = await startBundleSource(bundleRef, registry, eventSink, configDir, {
     allowInsecureRemotes: opts?.allowInsecureRemotes,
-    dataDir,
+    dataDir: preflightDataDir,
     // Thread workspace id + work dir so the named-bundle path can resolve
     // `user_config` from the workspace credential store before prepareServer
-    // validates it.
+    // validates it. Required for .mcpb too (workspace-scoped credentials).
     wsId,
     workDir,
   });
 
+  // For .mcpb, the manifest-derived name only became knowable after
+  // startBundleSource peeked the archive. Derive the canonical data-dir from
+  // it so re-uploads land in the same dir across restarts (matches the named
+  // branch convention).
+  const dataDir = isMcpb
+    ? resolveBundleDataDir(wsPath, result.sourceName)
+    : (preflightDataDir as string);
+
   return {
     wsId,
     bundle: bundleRef,
diff --git a/test/unit/mcpb-upload.test.ts b/test/unit/mcpb-upload.test.ts
index 6140823a..f3b23e56 100644
--- a/test/unit/mcpb-upload.test.ts
+++ b/test/unit/mcpb-upload.test.ts
@@ -432,27 +432,49 @@ describe("Fix 5 (continued): protected .mcpb gets internalEnv", () => {
 // The duplicate check uses the wrong name → silent collision.
 // ---------------------------------------------------------------------------
 
-describe("Fix 6: .mcpb server name must come from manifest, not file path", () => {
-	it("path-derived name must equal manifest-derived name for .mcpb", () => {
-		// serverNameFromRef({path: "/uploads/echo.mcpb"}) calls
-		// deriveServerName("/uploads/echo.mcpb")
-		const pathDerived = deriveServerName("/uploads/echo.mcpb");
-
-		// startBundleSource registers under deriveServerName(manifest.name)
-		// where manifest.name = "echo"
-		const manifestDerived = deriveServerName("echo");
-
-		// CORRECT: these must be equal so the duplicate check works
-		// Currently: pathDerived = "echo-mcpb", manifestDerived = "echo"
-		expect(pathDerived).toBe(manifestDerived);
+describe("Fix 6: install for .mcpb registers under manifest name, not path", () => {
+	beforeEach(() => {
+		capturedSpawnEnv = undefined;
+		mkdirSync(tmpCwd, { recursive: true });
+		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
+	});
+
+	afterEach(() => {
+		rmSync(tmpCwd, { recursive: true, force: true });
 	});
 
-	it("scoped name path-derived must equal manifest-derived", () => {
-		const pathDerived = deriveServerName("/uploads/@acme/my-tool.mcpb");
-		const manifestDerived = deriveServerName("my-tool");
+	it("install with .mcpb path returns manifest-derived sourceName", async () => {
+		const { installBundleInWorkspace } = await import(
+			"../../src/bundles/workspace-ops.ts"
+		);
+		const { ToolRegistry } = await import("../../src/tools/registry.ts");
+		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
 
-		// Both should resolve to "my-tool"
-		expect(pathDerived).toBe(manifestDerived);
+		const registry = new ToolRegistry();
+		const sink = new NoopEventSink();
+
+		// Pre-register a source under the WRONG (path-derived) name to prove
+		// the install pre-flight check no longer trips on it. With the bug,
+		// installBundleInWorkspace would compute `deriveServerName(path)` =
+		// "echo-mcpb", see no collision, then startBundleSource would try to
+		// register "echo" — the addSource throws but the pre-flight check
+		// failed to predict the right name.
+		const pathDerivedWrongName = deriveServerName("/uploads/echo.mcpb");
+		expect(pathDerivedWrongName).not.toBe("echo"); // sanity: they differ
+
+		const result = await installBundleInWorkspace(
+			"ws_test",
+			{ path: "/uploads/echo.mcpb" },
+			registry,
+			sink,
+			undefined,
+			{ workDir: "/tmp/nb-workdir" },
+		);
+
+		// CORRECT: registered under manifest name, not path-derived name
+		expect(result.serverName).toBe("echo");
+		expect(registry.hasSource("echo")).toBe(true);
+		expect(registry.hasSource(pathDerivedWrongName)).toBe(false);
 	});
 });
 

From e41c9fc7ad87df5d17c0cfb5fb28c425601f59c0 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 22:46:04 -0500
Subject: [PATCH 08/21] test: drop integration tests pending mpak-sdk@0.7.0
 release
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The earlier draft of this file used bun:test mock.module to stub
@nimblebrain/mpak-sdk, ../../src/config/workspace-credentials.ts,
../../src/tools/mcp-source.ts, and ../../src/bundles/mpak.ts so the
.mcpb branch of startBundleSource and the .mcpb-aware code in
installBundleInWorkspace could be exercised in isolation.

mock.module in bun:test is global within the test process. The stubs
leaked into every other test file in the same run (system-tools,
lifecycle, etc.), turning a clean main into 175+ failures. Confirmed
by isolating the file (clean) vs combining with system-tools.test.ts
(47 failures appeared).

Removes Fix 4, 5, 6 integration tests for now. Fix 1 (path traversal)
and Fix 3b (uninstall workspace.json filter) stay — pure logic, no
module mocks needed.

Once mpak-sdk@>=0.7.0 ships validateMcpb, integration coverage for
the deferred fixes can land in test/integration/ where each file runs
in its own process and real fixtures can drive end-to-end paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 test/unit/mcpb-upload.test.ts | 474 +++-------------------------------
 1 file changed, 42 insertions(+), 432 deletions(-)

diff --git a/test/unit/mcpb-upload.test.ts b/test/unit/mcpb-upload.test.ts
index f3b23e56..1a5b1fe0 100644
--- a/test/unit/mcpb-upload.test.ts
+++ b/test/unit/mcpb-upload.test.ts
@@ -1,29 +1,31 @@
 /**
  * Tests asserting CORRECT behavior for .mcpb bundle upload fixes.
- * These FAIL against the current (buggy) code and PASS after fixes.
  *
- * Covers PR #170 review issues:
- * 1. Path traversal in handleBundleUpload filename
- * 3a. Uninstall by path: server name resolution (path-derived ≠ manifest)
- * 3b. Uninstall by path: workspace.json filter must handle {path} entries
- * 4. .mcpb branch must call resolveUserConfig (workspace credentials)
- * 5. Missing MPAK_WORKSPACE / UPJACK_ROOT in .mcpb env
- * 6. Server name collision on install (path-derived ≠ manifest-derived)
+ * Covers PR #170 review issues that can be tested without mocking the
+ * `@nimblebrain/mpak-sdk` import surface:
  *
- * Fix 2 (SDK dep blocker) is a PR comment, not a code change — no test.
+ * - Fix 1: Path traversal in handleBundleUpload filename
+ * - Fix 3b: Uninstall workspace.json filter handles {path} entries
+ *
+ * Integration coverage for the remaining fixes (Fix 4, 5, 6 — startBundleSource
+ * .mcpb branch + installBundleInWorkspace .mcpb-awareness) is deferred until
+ * mpak-sdk@>=0.7.0 ships `validateMcpb`. Earlier drafts of this file used
+ * `mock.module` to stub the SDK, but bun:test's module mocks are global within
+ * the test process, so the stubs leaked across files and broke unrelated tests
+ * (system-tools, lifecycle, etc.). Once the SDK exports `validateMcpb`,
+ * integration tests for those fixes can land in `test/integration/` where
+ * isolation is cheaper and real mpak fixtures can drive end-to-end paths.
  */
 
-import { describe, expect, it, mock, beforeEach, afterEach } from "bun:test";
+import { describe, expect, it } from "bun:test";
 import { join } from "node:path";
-import { mkdirSync, writeFileSync, rmSync } from "node:fs";
-import { deriveServerName } from "../../src/bundles/paths.ts";
 
 // ---------------------------------------------------------------------------
 // Fix 1: Path traversal — uploaded filename must strip directory components
 //
-// handleBundleUpload does: join(bundlesDir, sanitizeFilename(filename))
+// handleBundleUpload joined sanitizeFilename(filename) onto bundlesDir.
 // sanitizeFilename only strips control chars / quotes, so "../../etc/foo.mcpb"
-// passes through unchanged → written path escapes bundlesDir.
+// passed through unchanged → written path escaped bundlesDir.
 //
 // Fix introduces an exported helper `safeBundleFilename` that the handler
 // uses to derive on-disk filename. Helper applies path.basename so all
@@ -37,9 +39,7 @@ describe("Fix 1: path traversal in handleBundleUpload", () => {
 			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
 		).safeBundleFilename;
 
-		// CORRECT: helper must be exported by handlers.ts
 		expect(typeof safeBundleFilename).toBe("function");
-		// CORRECT: traversal components stripped to plain filename
 		expect(safeBundleFilename!("../../etc/cron.daily/evil.mcpb")).toBe(
 			"evil.mcpb",
 		);
@@ -71,58 +71,40 @@ describe("Fix 1: path traversal in handleBundleUpload", () => {
 });
 
 // ---------------------------------------------------------------------------
-// Fix 3: Uninstall workspace.json filter must handle {path} entries
+// Fix 3b: uninstall workspace.json filter must match BOTH variants
 //
-// uninstallBundleFromWorkspaceViaCtx filters workspace.json bundles with:
-//   ws.bundles.filter((b) => !("name" in b && b.name === name))
-// This only removes {name} entries. {path} entries are never matched because
-// the target could be a path string, not a name string.
-// ---------------------------------------------------------------------------
-
-// ---------------------------------------------------------------------------
-// Fix 3a: uninstall by path — server name must come from manifest, not path
+// The previous filter took a single `target: string` and only matched
+// `{name}` entries — path-installed bundles became permanent residents of
+// workspace.json even after their tool source was deregistered. The new
+// filter takes `{name?, path?}` and dispatches per-variant.
 //
-// uninstallBundleFromWorkspaceViaCtx calls deriveServerName(name) where `name`
-// can be a `.mcpb` path string. For a path like "/uploads/echo.mcpb",
-// deriveServerName("/uploads/echo.mcpb") = "echo-mcpb". But the source was
-// registered under deriveServerName(manifest.name) = "echo".
-// → registry lookup misses, uninstall reports "no bundle found".
+// This is a unit test of the filter contract. Production wiring lives in
+// uninstallBundleFromWorkspaceViaCtx (system-tools.ts) and is exercised by
+// manual testing today; integration coverage tracks with the deferred Fix
+// 4/5/6 tests above.
 // ---------------------------------------------------------------------------
 
-describe("Fix 3a: uninstall by .mcpb path resolves to manifest-derived name", () => {
-	it("path-as-name derived must equal manifest-derived name", () => {
-		// Current uninstall: deriveServerName(name) where name = the path string
-		const uninstallDerived = deriveServerName("/uploads/echo.mcpb");
-		// Actual registered: deriveServerName(manifest.name) = "echo"
-		const registeredAs = deriveServerName("echo");
-		// CORRECT: uninstall must resolve to the manifest-derived registered name
-		expect(uninstallDerived).toBe(registeredAs);
-	});
-});
-
 describe("Fix 3b: uninstall workspace.json filter handles {path} entries", () => {
-	/**
-	 * Replicates the CURRENT (buggy) filter from uninstallBundleFromWorkspaceViaCtx.
-	 * It only matches by name — path entries slip through.
-	 */
-	function currentFilter(
+	function fixedFilter(
 		bundles: Array<{ name?: string; path?: string }>,
-		target: string,
+		target: { name?: string; path?: string },
 	): Array<{ name?: string; path?: string }> {
-		return bundles.filter((b) => !("name" in b && b.name === target));
+		return bundles.filter((b) => {
+			if (target.name && "name" in b) return b.name !== target.name;
+			if (target.path && "path" in b) return b.path !== target.path;
+			return true;
+		});
 	}
 
-	it("removing a path-based bundle must actually remove the {path} entry", () => {
+	it("removing a path-based bundle removes the {path} entry", () => {
 		const bundles = [
 			{ name: "@acme/hello" },
 			{ path: "/uploads/custom.mcpb" },
 			{ name: "@acme/world" },
 		];
 
-		// Uninstalling the path-based bundle by its path
-		const result = currentFilter(bundles, "/uploads/custom.mcpb");
+		const result = fixedFilter(bundles, { path: "/uploads/custom.mcpb" });
 
-		// CORRECT: should have 2 entries (path entry removed)
 		expect(result).toHaveLength(2);
 		expect(
 			result.some((b) => "path" in b && b.path === "/uploads/custom.mcpb"),
@@ -136,395 +118,23 @@ describe("Fix 3b: uninstall workspace.json filter handles {path} entries", () =>
 			{ name: "@acme/world" },
 		];
 
-		// Uninstalling a named bundle — this already works in current code
-		const result = currentFilter(bundles, "@acme/hello");
+		const result = fixedFilter(bundles, { name: "@acme/hello" });
 
 		expect(result).toHaveLength(2);
 		expect(
 			result.some((b) => "name" in b && b.name === "@acme/hello"),
 		).toBe(false);
 	});
-});
-
-// ---------------------------------------------------------------------------
-// Fix 5: .mcpb spawn env must include MPAK_WORKSPACE and UPJACK_ROOT
-//
-// The named-bundle branch in startBundleSource sets:
-//   MPAK_WORKSPACE: bundleDataDir, UPJACK_ROOT: bundleDataDir
-// The .mcpb branch omits these — bundles that depend on them break silently.
-//
-// Uses mock.module to intercept McpSource construction and capture spawn env.
-// ---------------------------------------------------------------------------
-
-let capturedSpawnEnv: Record<string, string> | undefined;
-let capturedResolveUserConfigOpts:
-	| { bundleName: string; userConfigSchema: unknown; wsId: string }
-	| undefined;
-let capturedPrepareServerOpts: { workspaceDir: string; userConfig?: unknown } | undefined;
-
-// validateMcpb ships in mpak-sdk@>=0.7.0 (mpak#94). Until that version is
-// published + dep-bumped, mock it here so the .mcpb branch is testable.
-mock.module("@nimblebrain/mpak-sdk", () => ({
-	validateMcpb: async (_path: string) => ({
-		valid: true,
-		manifest: testManifest,
-		errors: [],
-	}),
-}));
-
-mock.module("../../src/config/workspace-credentials.ts", () => ({
-	resolveUserConfig: async (opts: {
-		bundleName: string;
-		userConfigSchema: unknown;
-		wsId: string;
-		workDir: string;
-	}) => {
-		capturedResolveUserConfigOpts = {
-			bundleName: opts.bundleName,
-			userConfigSchema: opts.userConfigSchema,
-			wsId: opts.wsId,
-		};
-		return { api_key: "from-workspace-creds" };
-	},
-	friendlyMpakConfigError: (err: unknown) => err,
-	getWorkspaceCredentials: async () => ({}),
-	clearAllWorkspaceCredentials: async () => {},
-}));
-
-mock.module("../../src/tools/mcp-source.ts", () => ({
-	McpSource: class MockMcpSource {
-		name: string;
-		constructor(name: string, config: { type: string; spawn?: { env?: Record<string, string> } }) {
-			this.name = name;
-			if (config?.type === "stdio" && config.spawn?.env) {
-				capturedSpawnEnv = { ...config.spawn.env };
-			}
-		}
-		async start() {}
-		async stop() {}
-		async tools() {
-			return [];
-		}
-		async execute() {
-			return { content: [], isError: true as const };
-		}
-	},
-}));
-
-mock.module("../../src/bundles/mpak.ts", () => ({
-	getMpak: () => ({
-		prepareServer: async (
-			_ref: unknown,
-			opts: { workspaceDir: string; userConfig?: unknown },
-		) => {
-			capturedPrepareServerOpts = opts;
-			return {
-				command: "node",
-				args: ["index.js"],
-				env: { MPAK_SDK_VAR: "from-sdk" },
-				cwd: "/tmp/nb-test-mcpb-env",
-				name: "echo",
-			};
-		},
-		bundleCache: {
-			getBundleManifest: () => null,
-		},
-	}),
-}));
-
-const tmpCwd = "/tmp/nb-test-mcpb-env";
-const testManifest = {
-	name: "echo",
-	version: "1.0.0",
-	description: "Test bundle",
-	user_config: {
-		api_key: { type: "string", title: "API Key", required: true },
-	},
-	server: {
-		type: "node",
-		mcp_config: {
-			command: "node",
-			args: ["index.js"],
-			env: { ECHO_API_KEY: "${user_config.api_key}" },
-		},
-	},
-};
-
-async function runStartBundleSourceForMcpb() {
-	const { startBundleSource } = await import("../../src/bundles/startup.ts");
-	const { ToolRegistry } = await import("../../src/tools/registry.ts");
-	const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
-
-	const registry = new ToolRegistry();
-	const sink = new NoopEventSink();
-	const ref = { path: "/test/echo.mcpb" };
-
-	await startBundleSource(ref, registry, sink, undefined, {
-		wsId: "ws_test",
-		workDir: "/tmp/nb-workdir",
-		dataDir: "/tmp/nb-workdir/data/echo",
-	});
-}
-
-describe("Fix 5: .mcpb spawn env includes MPAK_WORKSPACE and UPJACK_ROOT", () => {
-	beforeEach(() => {
-		capturedSpawnEnv = undefined;
-		capturedResolveUserConfigOpts = undefined;
-		capturedPrepareServerOpts = undefined;
-		mkdirSync(tmpCwd, { recursive: true });
-		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
-	});
-
-	afterEach(() => {
-		rmSync(tmpCwd, { recursive: true, force: true });
-	});
-
-	it("spawn env must include MPAK_WORKSPACE and UPJACK_ROOT", async () => {
-		await runStartBundleSourceForMcpb();
-
-		expect(capturedSpawnEnv).toBeDefined();
-		// CORRECT: both env vars must be set to the data dir
-		expect(capturedSpawnEnv!.MPAK_WORKSPACE).toBe("/tmp/nb-workdir/data/echo");
-		expect(capturedSpawnEnv!.UPJACK_ROOT).toBe("/tmp/nb-workdir/data/echo");
-	});
-});
-
-// ---------------------------------------------------------------------------
-// Fix 4: .mcpb branch must call resolveUserConfig and pass userConfig
-//
-// The named-bundle branch resolves workspace credentials via resolveUserConfig
-// before calling prepareServer. The .mcpb branch skips this entirely → bundles
-// declaring user_config never see workspace-stored credentials, and the SDK's
-// missing-required check fires even when creds are configured.
-// ---------------------------------------------------------------------------
-
-describe("Fix 4: .mcpb branch resolves workspace credentials", () => {
-	beforeEach(() => {
-		capturedSpawnEnv = undefined;
-		capturedResolveUserConfigOpts = undefined;
-		capturedPrepareServerOpts = undefined;
-		mkdirSync(tmpCwd, { recursive: true });
-		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
-	});
-
-	afterEach(() => {
-		rmSync(tmpCwd, { recursive: true, force: true });
-	});
-
-	it("resolveUserConfig must be called with manifest name and user_config schema", async () => {
-		await runStartBundleSourceForMcpb();
-
-		expect(capturedResolveUserConfigOpts).toBeDefined();
-		// Must use manifest-derived bundle name, not the file path
-		expect(capturedResolveUserConfigOpts!.bundleName).toBe("echo");
-		expect(capturedResolveUserConfigOpts!.wsId).toBe("ws_test");
-		// Must thread the manifest's user_config schema for credential resolution
-		expect(capturedResolveUserConfigOpts!.userConfigSchema).toEqual(
-			testManifest.user_config,
-		);
-	});
-
-	it("resolved userConfig must be passed to prepareServer", async () => {
-		await runStartBundleSourceForMcpb();
-
-		expect(capturedPrepareServerOpts).toBeDefined();
-		// CORRECT: resolved creds must reach the SDK's prepareServer
-		expect(capturedPrepareServerOpts!.userConfig).toEqual({
-			api_key: "from-workspace-creds",
-		});
-	});
-});
-
-// ---------------------------------------------------------------------------
-// Fix 4 (continued): .mcpb branch must require opts.wsId
-//
-// The named-bundle branch hard-errors when wsId is missing (workspace-scoped
-// credential resolution requires it). The .mcpb branch must do the same —
-// silently defaulting would pool credentials across tenants.
-// ---------------------------------------------------------------------------
-
-describe("Fix 4 (continued): .mcpb branch requires opts.wsId", () => {
-	beforeEach(() => {
-		capturedSpawnEnv = undefined;
-		mkdirSync(tmpCwd, { recursive: true });
-		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
-	});
 
-	afterEach(() => {
-		rmSync(tmpCwd, { recursive: true, force: true });
-	});
-
-	it("startBundleSource for .mcpb throws when wsId missing", async () => {
-		const { startBundleSource } = await import("../../src/bundles/startup.ts");
-		const { ToolRegistry } = await import("../../src/tools/registry.ts");
-		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
-
-		const registry = new ToolRegistry();
-		const sink = new NoopEventSink();
-		const ref = { path: "/test/echo.mcpb" };
-
-		// CORRECT: must throw because workspace-scoped credentials cannot resolve
-		// without wsId. Currently passes silently — credentials never resolve.
-		await expect(
-			startBundleSource(ref, registry, sink, undefined, {
-				workDir: "/tmp/nb-workdir",
-			}),
-		).rejects.toThrow(/workspace/i);
-	});
-});
-
-// ---------------------------------------------------------------------------
-// Fix 5 (continued): protected .mcpb spawn env includes internalEnv
-//
-// Named/local branches inject NB_INTERNAL_TOKEN + NB_HOST_URL into spawn env
-// when ref.protected is true and opts.internalEnv is provided. The .mcpb
-// branch never threads internalEnv → protected bundles cannot reach the host.
-// ---------------------------------------------------------------------------
-
-describe("Fix 5 (continued): protected .mcpb gets internalEnv", () => {
-	beforeEach(() => {
-		capturedSpawnEnv = undefined;
-		mkdirSync(tmpCwd, { recursive: true });
-		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
-	});
-
-	afterEach(() => {
-		rmSync(tmpCwd, { recursive: true, force: true });
-	});
-
-	it("spawn env must include NB_INTERNAL_TOKEN and NB_HOST_URL when ref.protected", async () => {
-		const { startBundleSource } = await import("../../src/bundles/startup.ts");
-		const { ToolRegistry } = await import("../../src/tools/registry.ts");
-		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
-
-		const registry = new ToolRegistry();
-		const sink = new NoopEventSink();
-		const ref = { path: "/test/echo.mcpb", protected: true };
-
-		await startBundleSource(ref, registry, sink, undefined, {
-			wsId: "ws_test",
-			workDir: "/tmp/nb-workdir",
-			dataDir: "/tmp/nb-workdir/data/echo",
-			internalEnv: {
-				NB_INTERNAL_TOKEN: "secret-token",
-				NB_HOST_URL: "http://localhost:27247",
-			},
-		});
-
-		expect(capturedSpawnEnv).toBeDefined();
-		// CORRECT: protected .mcpb must receive host-comm credentials
-		expect(capturedSpawnEnv!.NB_INTERNAL_TOKEN).toBe("secret-token");
-		expect(capturedSpawnEnv!.NB_HOST_URL).toBe("http://localhost:27247");
-	});
-});
-
-// ---------------------------------------------------------------------------
-// Fix 6: Server name collision — path-derived ≠ manifest-derived
-//
-// workspace-ops.ts serverNameFromRef uses deriveServerName(ref.path) for path
-// refs. But startBundleSource registers .mcpb bundles under
-// deriveServerName(manifest.name). These differ because the path includes
-// the ".mcpb" extension: deriveServerName("echo.mcpb") → "echo-mcpb",
-// but deriveServerName("echo") → "echo".
-//
-// installBundleInWorkspace pre-computes the server name from the path for its
-// duplicate check, but the actual registered name comes from the manifest.
-// The duplicate check uses the wrong name → silent collision.
-// ---------------------------------------------------------------------------
-
-describe("Fix 6: install for .mcpb registers under manifest name, not path", () => {
-	beforeEach(() => {
-		capturedSpawnEnv = undefined;
-		mkdirSync(tmpCwd, { recursive: true });
-		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
-	});
-
-	afterEach(() => {
-		rmSync(tmpCwd, { recursive: true, force: true });
-	});
-
-	it("install with .mcpb path returns manifest-derived sourceName", async () => {
-		const { installBundleInWorkspace } = await import(
-			"../../src/bundles/workspace-ops.ts"
-		);
-		const { ToolRegistry } = await import("../../src/tools/registry.ts");
-		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
-
-		const registry = new ToolRegistry();
-		const sink = new NoopEventSink();
-
-		// Pre-register a source under the WRONG (path-derived) name to prove
-		// the install pre-flight check no longer trips on it. With the bug,
-		// installBundleInWorkspace would compute `deriveServerName(path)` =
-		// "echo-mcpb", see no collision, then startBundleSource would try to
-		// register "echo" — the addSource throws but the pre-flight check
-		// failed to predict the right name.
-		const pathDerivedWrongName = deriveServerName("/uploads/echo.mcpb");
-		expect(pathDerivedWrongName).not.toBe("echo"); // sanity: they differ
-
-		const result = await installBundleInWorkspace(
-			"ws_test",
+	it("name-targeted filter does not accidentally match path entries", () => {
+		const bundles = [
 			{ path: "/uploads/echo.mcpb" },
-			registry,
-			sink,
-			undefined,
-			{ workDir: "/tmp/nb-workdir" },
-		);
-
-		// CORRECT: registered under manifest name, not path-derived name
-		expect(result.serverName).toBe("echo");
-		expect(registry.hasSource("echo")).toBe(true);
-		expect(registry.hasSource(pathDerivedWrongName)).toBe(false);
-	});
-});
-
-// ---------------------------------------------------------------------------
-// Fix 6 (continued): installBundleInWorkspace returns dataDir from manifest
-//
-// installBundleInWorkspace currently computes dataDir from
-// `bundleNameFromRef(ref) = ref.path` for path refs. For .mcpb,
-// `deriveBundleDataDir("/uploads/echo.mcpb")` produces a non-portable
-// path-derived dir. After fix, dataDir comes from manifest name.
-// ---------------------------------------------------------------------------
-
-describe("Fix 6 (continued): install returns manifest-derived dataDir", () => {
-	beforeEach(() => {
-		capturedSpawnEnv = undefined;
-		mkdirSync(tmpCwd, { recursive: true });
-		writeFileSync(join(tmpCwd, "manifest.json"), JSON.stringify(testManifest));
-	});
-
-	afterEach(() => {
-		rmSync(tmpCwd, { recursive: true, force: true });
-	});
-
-	it("install with .mcpb path returns dataDir derived from manifest, not path", async () => {
-		const { installBundleInWorkspace } = await import(
-			"../../src/bundles/workspace-ops.ts"
-		);
-		const { ToolRegistry } = await import("../../src/tools/registry.ts");
-		const { NoopEventSink } = await import("../../src/adapters/noop-events.ts");
-
-		const registry = new ToolRegistry();
-		const sink = new NoopEventSink();
+			{ name: "/uploads/echo.mcpb" },
+		];
 
-		const result = await installBundleInWorkspace(
-			"ws_test",
-			{ path: "/uploads/echo.mcpb" },
-			registry,
-			sink,
-			undefined,
-			{ workDir: "/tmp/nb-workdir" },
-		);
+		const result = fixedFilter(bundles, { name: "/uploads/echo.mcpb" });
 
-		// CORRECT: serverName from manifest
-		expect(result.serverName).toBe("echo");
-		// CORRECT: dataDir from manifest name, portable across re-uploads
-		expect(result.dataDir).toBe(
-			join("/tmp/nb-workdir/workspaces/ws_test/data", "echo"),
-		);
-		// Negative: must NOT contain path components
-		expect(result.dataDir).not.toContain("uploads");
-		expect(result.dataDir).not.toContain("mcpb");
+		expect(result).toHaveLength(1);
+		expect(result[0]).toEqual({ path: "/uploads/echo.mcpb" });
 	});
 });

From c371c60d486086ebcdaf92a775c26ba64da6d0a2 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 22:46:17 -0500
Subject: [PATCH 09/21] fix: thread .mcpb path through uninstall, repair
 workspace.json filter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

uninstallBundleFromWorkspaceViaCtx took a single `target: string`
parameter and called deriveServerName(target). For .mcpb path
uninstalls (target = "/uploads/echo.mcpb"), this produced
"echo-mcpb" — but the source was registered under deriveServerName
(manifest.name) = "echo". Three downstream failures:

1. lifecycle.getInstance(serverName, wsId) returned nothing because
   it was looking up the wrong key. The protected-bundle check
   silently passed and uninstall proceeded for protected bundles.
2. uninstallBundleFromWorkspace(wsId, target, registry) hit the
   registry under the wrong name → "no bundle found" with no hint
   that the path argument was the cause.
3. The workspace.json filter only matched {name} entries:
     ws.bundles.filter((b) => !("name" in b && b.name === name))
   Path-installed bundles became permanent residents of
   workspace.json even after their tool source was deregistered.

Fix: change the function signature to accept { name?, path? },
mirroring the install handler. For .mcpb path uninstalls, peek the
manifest via validateMcpb(path) before calling deriveServerName so
every downstream lookup uses the manifest-derived name. Update the
workspace.json filter to dispatch per-variant — match {name} when
target.name is set, {path} when target.path is set.

manage_app handler now passes { name, path } through unchanged
(matches the install side).

Closes PR #170 review comment 3.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/tools/system-tools.ts | 78 ++++++++++++++++++++++++++++++++-------
 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/src/tools/system-tools.ts b/src/tools/system-tools.ts
index 2a9eff7e..cc9e3581 100644
--- a/src/tools/system-tools.ts
+++ b/src/tools/system-tools.ts
@@ -219,15 +219,14 @@ export async function createSystemTools(
           );
         }
         if (action === "uninstall") {
-          const target = name ?? path;
-          if (!target) {
+          if (!name && !path) {
             return {
               content: textContent("Either 'name' or 'path' is required for uninstall"),
               isError: true,
             };
           }
           return await uninstallBundleFromWorkspaceViaCtx(
-            target,
+            { name, path },
             wsId,
             lifecycle,
             getRegistry(),
@@ -874,10 +873,7 @@ async function installBundleInWorkspaceViaCtx(
 
     const ws = await ctx.workspaceStore.get(wsId);
     if (ws) {
-      const already = ws.bundles.some((b) => {
-        if (target.name) return "name" in b && b.name === target.name;
-        return "path" in b && b.path === target.path;
-      });
+      const already = ws.bundles.some((b) => bundleEntryMatchesTarget(b, target));
       if (!already) {
         await ctx.workspaceStore.update(wsId, {
           bundles: [...ws.bundles, target.name ? { name: target.name } : { path: target.path! }],
@@ -922,6 +918,27 @@ async function installBundleInWorkspaceViaCtx(
  * predate `serverName`-on-ref persistence. Exported so the regression
  * is unit-testable independently of the full uninstall stack.
  */
+/**
+ * True if a persisted workspace.json `BundleRef` entry matches the
+ * `{name?, path?}` target shape used by the install/uninstall handlers.
+ *
+ * One predicate, two call sites — the install pre-check ("is this bundle
+ * already registered in workspace.json?") and the uninstall filter ("which
+ * entry should I drop?"). Keeping the dispatch in one place means the two
+ * stay in lockstep; the previous filter only matched `{name}` entries and
+ * silently left `{path}`-installed bundles in workspace.json forever.
+ *
+ * Exported so the unit test pins the contract.
+ */
+export function bundleEntryMatchesTarget(
+  entry: { name?: string; path?: string },
+  target: { name?: string; path?: string },
+): boolean {
+  if (target.name && "name" in entry) return entry.name === target.name;
+  if (target.path && "path" in entry) return entry.path === target.path;
+  return false;
+}
+
 export function resolveBundleServerName(
   bundleName: string,
   ws: { bundles: Array<{ name?: string; serverName?: string }> } | null,
@@ -931,15 +948,46 @@ export function resolveBundleServerName(
 }
 
 async function uninstallBundleFromWorkspaceViaCtx(
-  name: string,
+  target: { name?: string; path?: string },
   wsId: string,
   lifecycle: BundleLifecycleManager,
   registry: ToolRegistry,
   ctx: ManageBundleContext,
 ): Promise<ToolResult> {
+  const label = target.name ?? target.path!;
   try {
+    // Resolve the bundle name. For .mcpb path uninstalls the manifest name
+    // lives inside the archive — peek via validateMcpb so the downstream
+    // lookups line up with what install registered. Otherwise deriveServerName
+    // (path) would produce "echo-mcpb" while the source is registered as
+    // "echo", and the protected check, registry deregister, and
+    // workspace.json filter all silently miss.
+    let bundleName: string;
+    if (target.name) {
+      bundleName = target.name;
+    } else if (target.path?.endsWith(".mcpb")) {
+      const { validateMcpb } = await import("@nimblebrain/mpak-sdk");
+      const validation = await validateMcpb(target.path);
+      if (!validation.valid || !validation.manifest) {
+        throw new Error(
+          `Cannot read manifest from ${target.path}: ${validation.errors?.join(", ") ?? "validation failed"}`,
+        );
+      }
+      bundleName = validation.manifest.name;
+    } else {
+      // Plain local-dir path (rare uninstall surface) — the path doubles as
+      // the bundle-name input. resolveBundleServerName + the {path} branch of
+      // the workspace.json filter both handle this shape.
+      bundleName = target.path!;
+    }
+
+    // Read the persisted slug-form `serverName` off the workspace entry
+    // (set at install time from `slugifyServerName(entry.id)`) with
+    // `deriveServerName(bundleName)` as the legacy fallback. Re-deriving
+    // here would compute the OLD short slug and miss any source registered
+    // under the post-#195 canonical-id slug.
     const ws = await ctx.workspaceStore.get(wsId);
-    const serverName = resolveBundleServerName(name, ws);
+    const serverName = resolveBundleServerName(bundleName, ws);
 
     // Protected check — pass wsId to look up the workspace-scoped instance
     const instance = lifecycle.getInstance(serverName, wsId);
@@ -950,7 +998,7 @@ async function uninstallBundleFromWorkspaceViaCtx(
     // Stop process and deregister from tool registry. Thread workDir so
     // the workspace credential file for this bundle is cleaned up as part
     // of uninstall (best-effort inside uninstallBundleFromWorkspace).
-    await uninstallBundleFromWorkspace(wsId, name, serverName, registry, {
+    await uninstallBundleFromWorkspace(wsId, bundleName, serverName, registry, {
       workDir: ctx.workDir,
     });
 
@@ -960,10 +1008,14 @@ async function uninstallBundleFromWorkspaceViaCtx(
     }
     lifecycle.removeInstance(serverName, wsId);
 
-    // Remove bundle from workspace.json
+    // Remove bundle from workspace.json. Filter dispatches per-variant —
+    // {name} when uninstalling by name, {path} when uninstalling by path.
+    // The previous filter only checked "name" in b, so path-installed
+    // bundles became permanent residents of workspace.json even after
+    // their tool source was deregistered.
     if (ws) {
       await ctx.workspaceStore.update(wsId, {
-        bundles: ws.bundles.filter((b) => !("name" in b && b.name === name)),
+        bundles: ws.bundles.filter((b) => !bundleEntryMatchesTarget(b, target)),
       });
     }
 
@@ -974,7 +1026,7 @@ async function uninstallBundleFromWorkspaceViaCtx(
   } catch (err) {
     return {
       content: textContent(
-        `Failed to uninstall ${name} from workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`,
+        `Failed to uninstall ${label} from workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`,
       ),
       isError: true,
     };

From a5ced45fb9ae4b344b57b0d462d67efa5e796a9b Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Tue, 5 May 2026 23:15:06 -0500
Subject: [PATCH 10/21] fix(types): infer Request.formData() return type
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Annotating the variable as `FormData` clashed in CI: Bun's
Request.formData() returns the undici `FormData`, but the explicit
`FormData` annotation resolved to the DOM-lib type from a different
ambient declaration. Their iterator types differ (FormDataEntryValue vs
string), so the assignment failed strict typecheck.

Inferring via `Awaited<ReturnType<typeof raw.formData>>` keeps the
variable strongly typed without pinning it to the wrong global.

Surfaced by PR #170 CI run 25415571865; the validateMcpb TS errors in
the same run are intentionally left in place — they document that the
PR is blocked on @nimblebrain/mpak-sdk@>=0.7.0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/api/handlers.ts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index 3e8cd49d..f9564677 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1398,7 +1398,11 @@ export async function handleBundleUpload(
   runtime: Runtime,
   workspaceId: string,
 ): Promise<Response> {
-  let formData: FormData;
+  // Inferred type rather than `FormData` annotation — Bun's Request.formData()
+  // returns the undici FormData, which has incompatible iterator types with
+  // the DOM-lib `FormData` resolved at the annotation site. They're shape-
+  // compatible at runtime; let TS infer to avoid the cross-type assignment.
+  let formData: Awaited<ReturnType<typeof raw.formData>>;
   try {
     formData = await raw.formData();
   } catch {

From 1c6c6ca1c442a9f3cd22d3bf4f7397b417b151aa Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:09:32 -0500
Subject: [PATCH 11/21] refactor(api): hoist node:fs and mpak-sdk imports in
 handleBundleUpload
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Dynamic imports inside the handler offered no benefit — handlers.ts loads
on every API boot, so deferring only shifts the cost to first-call latency
without avoiding it. Hoists `mkdirSync`, `writeFileSync`, `unlinkSync` from
`node:fs` and `validateMcpb` from `@nimblebrain/mpak-sdk` to module scope.
`join` already top-level imported; drops the local `joinPath` alias.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/api/handlers.ts | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index f9564677..8343e412 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1,5 +1,6 @@
-import { readFileSync } from "node:fs";
+import { mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";
 import { basename, join, resolve } from "node:path";
+import { validateMcpb } from "@nimblebrain/mpak-sdk";
 import { CallbackEventSink } from "../adapters/callback-events.ts";
 import { log } from "../cli/log.ts";
 import { isToolEnabled, isToolVisibleToRole, type ResolvedFeatures } from "../config/features.ts";
@@ -1438,23 +1439,18 @@ export async function handleBundleUpload(
   }
 
   // Save to workspace bundles directory
-  const { mkdirSync, writeFileSync } = await import("node:fs");
-  const { join: joinPath } = await import("node:path");
-
-  const bundlesDir = joinPath(runtime.getWorkspaceScopedDir(workspaceId), "bundles");
+  const bundlesDir = join(runtime.getWorkspaceScopedDir(workspaceId), "bundles");
   mkdirSync(bundlesDir, { recursive: true });
 
   const safeName = safeBundleFilename(filename);
-  const bundlePath = joinPath(bundlesDir, safeName);
+  const bundlePath = join(bundlesDir, safeName);
   writeFileSync(bundlePath, data, { mode: 0o600 });
 
   // Validate via mpak SDK
-  const { validateMcpb } = await import("@nimblebrain/mpak-sdk");
   const result = await validateMcpb(bundlePath);
 
   if (!result.valid) {
     // Clean up invalid bundle
-    const { unlinkSync } = await import("node:fs");
     try {
       unlinkSync(bundlePath);
     } catch {

From bd8bfed1a6134cbbbc103a725d0387fe9b0082ab Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:11:46 -0500
Subject: [PATCH 12/21] fix(bundles): confine manage_app.path to workspace
 bundles dir
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

LLM-supplied `path` to `manage_app` was unrestricted — a successful prompt
injection (chat content, RAG doc, scraped page) could pass any local
`.mcpb` archive to install/uninstall. The install path then spawned a
subprocess under the platform user with workspace credentials,
`MPAK_WORKSPACE`, `UPJACK_ROOT`, and (for protected refs) `NB_INTERNAL_TOKEN`
attached, plus persisted the path to `workspace.json` so it re-spawned every
restart.

Adds `assertPathInWorkspaceBundlesDir` to `src/bundles/paths.ts`. Realpaths
both sides where possible so symlinks pointing outside the dir are rejected.
Falls back to a lexical resolve when the file does not exist (uninstall
after manual deletion is allowed; the persisted path is already trusted).

Wires the check into three call sites:

- `installBundleInWorkspaceViaCtx` — primary control. Rejects bad path
  before constructing the bundle ref so `startBundleSource` never sees it.
- `uninstallBundleFromWorkspaceViaCtx` — mirror guard. Without it, an
  attacker could pass an arbitrary path to read its manifest via
  `validateMcpb` (which extracts and parses the archive).
- `startBundleSource` `.mcpb` branch — defense-in-depth at re-hydration.
  Catches a tampered or out-of-band-edited `workspace.json`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/bundles/paths.ts      | 50 ++++++++++++++++++++++++++++++++++++++-
 src/bundles/startup.ts    |  8 +++++++
 src/tools/system-tools.ts | 39 +++++++++++++++++++++++++++++-
 3 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/src/bundles/paths.ts b/src/bundles/paths.ts
index 7f699dc1..5a04fa56 100644
--- a/src/bundles/paths.ts
+++ b/src/bundles/paths.ts
@@ -1,4 +1,5 @@
-import { join } from "node:path";
+import { existsSync, realpathSync } from "node:fs";
+import { join, resolve, sep } from "node:path";
 import type { BundleRef } from "./types.ts";
 
 /** Prefixes reserved for system tools — bundles must not use these as source names. */
@@ -102,3 +103,50 @@ export function deriveBundleDataDir(name: string): string {
 export function resolveBundleDataDir(workspacePath: string, bundleName: string): string {
   return join(workspacePath, "data", deriveBundleDataDir(bundleName));
 }
+
+/**
+ * Resolve the absolute on-disk path of the workspace bundles directory —
+ * the only location an LLM-supplied `.mcpb` path is permitted to reference.
+ */
+export function resolveWorkspaceBundlesDir(workDir: string, wsId: string): string {
+  return join(workDir, "workspaces", wsId, "bundles");
+}
+
+/**
+ * Assert that `filePath` resolves inside `<workDir>/workspaces/<wsId>/bundles/`.
+ * Throws otherwise.
+ *
+ * Critical defense for the `manage_app({ path })` tool path. Without this
+ * guard, prompt-injected agent input can install any `.mcpb` the platform
+ * user can read — the spawned subprocess inherits workspace credentials and
+ * (for protected bundles) `NB_INTERNAL_TOKEN`. The same check runs at
+ * startup re-hydration so a tampered `workspace.json` cannot escape either.
+ *
+ * Both sides are realpath'd when possible so symlinks pointing outside the
+ * bundles dir are rejected. If the file does not exist (uninstall after
+ * manual deletion), falls back to a lexical resolve — workspace.json is
+ * written by trusted code, so a missing file is not a sign of tampering.
+ */
+export function assertPathInWorkspaceBundlesDir(
+  filePath: string,
+  workDir: string,
+  wsId: string,
+): void {
+  const bundlesDir = resolveWorkspaceBundlesDir(workDir, wsId);
+  // Realpath the bundles dir if it exists; otherwise fall back to a lexical
+  // resolve. The dir is created on first upload, so a missing dir means no
+  // bundle could possibly live inside it — a guard violation either way.
+  const canonicalBundlesDir = existsSync(bundlesDir)
+    ? realpathSync(bundlesDir)
+    : resolve(bundlesDir);
+  const canonicalFile = existsSync(filePath) ? realpathSync(filePath) : resolve(filePath);
+  if (
+    canonicalFile !== canonicalBundlesDir &&
+    !canonicalFile.startsWith(canonicalBundlesDir + sep)
+  ) {
+    throw new Error(
+      `Bundle path must live inside the workspace bundles directory ` +
+        `(${canonicalBundlesDir}); got ${canonicalFile}`,
+    );
+  }
+}
diff --git a/src/bundles/startup.ts b/src/bundles/startup.ts
index 8807121c..c9ec8889 100644
--- a/src/bundles/startup.ts
+++ b/src/bundles/startup.ts
@@ -21,6 +21,7 @@ import { filterEnvForBundle } from "./env-filter.ts";
 import { validateManifest } from "./manifest.ts";
 import { getMpak } from "./mpak.ts";
 import {
+  assertPathInWorkspaceBundlesDir,
   deriveBundleDataDir,
   deriveServerName,
   resolveBundleDataDir,
@@ -505,6 +506,13 @@ export async function startBundleSource(
     }
 
     const nbWorkDir = opts.workDir ?? process.env.NB_WORK_DIR ?? join(homedir(), ".nimblebrain");
+
+    // Defense-in-depth: re-check at re-hydration that the persisted path
+    // still lives inside the workspace bundles dir. The install-side guard
+    // in `manage_app` is the primary control; this catches a tampered or
+    // out-of-band-edited `workspace.json`.
+    assertPathInWorkspaceBundlesDir(ref.path, nbWorkDir, opts.wsId);
+
     const mpakHome = process.env.MPAK_HOME ?? join(homedir(), ".mpak");
     const mpak = getMpak(mpakHome);
 
diff --git a/src/tools/system-tools.ts b/src/tools/system-tools.ts
index cc9e3581..9f8b53e8 100644
--- a/src/tools/system-tools.ts
+++ b/src/tools/system-tools.ts
@@ -1,7 +1,7 @@
 import { NoopEventSink } from "../adapters/noop-events.ts";
 import type { BundleLifecycleManager } from "../bundles/lifecycle.ts";
 import { getMpak } from "../bundles/mpak.ts";
-import { deriveServerName } from "../bundles/paths.ts";
+import { assertPathInWorkspaceBundlesDir, deriveServerName } from "../bundles/paths.ts";
 import { startBundleSource } from "../bundles/startup.ts";
 import type { BundleManifest } from "../bundles/types.ts";
 import {
@@ -841,6 +841,24 @@ async function installBundleInWorkspaceViaCtx(
   ctx: ManageBundleContext,
 ): Promise<ToolResult> {
   const label = target.name ?? target.path!;
+
+  // Confine LLM-supplied `path` to the workspace bundles dir — see
+  // `assertPathInWorkspaceBundlesDir`. Reject before constructing the ref so
+  // a malicious path never reaches `startBundleSource` (which would spawn a
+  // subprocess with workspace credentials).
+  if (target.path) {
+    try {
+      assertPathInWorkspaceBundlesDir(target.path, ctx.workDir, wsId);
+    } catch (err) {
+      return {
+        content: textContent(
+          `Failed to install ${label} in workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`,
+        ),
+        isError: true,
+      };
+    }
+  }
+
   const bundleRef = (
     target.name ? { name: target.name } : { path: target.path! }
   ) as import("../bundles/types.ts").BundleRef;
@@ -955,6 +973,25 @@ async function uninstallBundleFromWorkspaceViaCtx(
   ctx: ManageBundleContext,
 ): Promise<ToolResult> {
   const label = target.name ?? target.path!;
+
+  // Mirror the install-side guard. An attacker who can poison the agent's
+  // tool input could otherwise pass an arbitrary `path` to read its manifest
+  // (validateMcpb extracts and parses the archive). Confining to the
+  // workspace bundles dir bounds the surface to artifacts the workspace
+  // already accepted via upload.
+  if (target.path) {
+    try {
+      assertPathInWorkspaceBundlesDir(target.path, ctx.workDir, wsId);
+    } catch (err) {
+      return {
+        content: textContent(
+          `Failed to uninstall ${label} from workspace ${wsId}: ${err instanceof Error ? err.message : String(err)}`,
+        ),
+        isError: true,
+      };
+    }
+  }
+
   try {
     // Resolve the bundle name. For .mcpb path uninstalls the manifest name
     // lives inside the archive — peek via validateMcpb so the downstream

From 9fcd4c85a8e4b8d63562561a8da4b292991146d8 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:12:23 -0500
Subject: [PATCH 13/21] fix(api): validate uploaded .mcpb in tempfile before
 commit to bundles dir
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`handleBundleUpload` previously wrote the upload directly into the
workspace bundles dir and only unlinked it on validation failure (best
effort). If unlink failed (perm/race) or the process crashed between write
and unlink, an unvalidated `.mcpb` lingered in the bundles dir — a stale
artifact the install path could later spawn.

Switches to a validate-then-commit flow:

1. Write upload to a random tempfile under `os.tmpdir()`.
2. Run `validateMcpb` against the temp path. On failure or thrown error,
   unlink temp and return.
3. On success, `renameSync` the temp into the bundles dir.

Keeps the bundles dir to validated content only, regardless of crash
timing. Rename is atomic on a shared filesystem; cross-fs falls back to
copy + unlink — acceptable since the archive is already trusted.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/api/handlers.ts | 50 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index 8343e412..acb34e02 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1,4 +1,6 @@
-import { mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";
+import { randomBytes } from "node:crypto";
+import { mkdirSync, readFileSync, renameSync, unlinkSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
 import { basename, join, resolve } from "node:path";
 import { validateMcpb } from "@nimblebrain/mpak-sdk";
 import { CallbackEventSink } from "../adapters/callback-events.ts";
@@ -1438,21 +1440,40 @@ export async function handleBundleUpload(
     return apiError(400, "bad_request", "Uploaded file is empty");
   }
 
-  // Save to workspace bundles directory
+  // Validate-then-commit:
+  //
+  // Write the upload to a tempfile under the OS temp dir first, run
+  // `validateMcpb` against it, and only `rename` into the workspace bundles
+  // dir on success. The previous order (write into the bundles dir, then
+  // validate, then unlink on failure) leaked partially-trusted artifacts:
+  // if unlink failed (perm/race) or the process crashed between write and
+  // unlink, an unvalidated `.mcpb` lingered in the bundles dir — a stale
+  // file the install path could later spawn. Tempfile + rename keeps the
+  // bundles dir to validated content only.
   const bundlesDir = join(runtime.getWorkspaceScopedDir(workspaceId), "bundles");
   mkdirSync(bundlesDir, { recursive: true });
 
   const safeName = safeBundleFilename(filename);
+  const tempPath = join(tmpdir(), `nb-mcpb-${randomBytes(8).toString("hex")}.mcpb`);
   const bundlePath = join(bundlesDir, safeName);
-  writeFileSync(bundlePath, data, { mode: 0o600 });
 
-  // Validate via mpak SDK
-  const result = await validateMcpb(bundlePath);
+  writeFileSync(tempPath, data, { mode: 0o600 });
+
+  let result: Awaited<ReturnType<typeof validateMcpb>>;
+  try {
+    result = await validateMcpb(tempPath);
+  } catch (err) {
+    try {
+      unlinkSync(tempPath);
+    } catch {
+      // best-effort cleanup
+    }
+    throw err;
+  }
 
   if (!result.valid) {
-    // Clean up invalid bundle
     try {
-      unlinkSync(bundlePath);
+      unlinkSync(tempPath);
     } catch {
       // best-effort cleanup
     }
@@ -1461,6 +1482,21 @@ export async function handleBundleUpload(
     });
   }
 
+  // Validation passed — promote tempfile into the workspace bundles dir.
+  // `renameSync` is atomic when source and destination share a filesystem;
+  // when they don't (tmpdir on a separate fs), Node falls back to copy +
+  // unlink, which is good enough — we already hold a valid archive.
+  try {
+    renameSync(tempPath, bundlePath);
+  } catch (err) {
+    try {
+      unlinkSync(tempPath);
+    } catch {
+      // best-effort cleanup
+    }
+    throw err;
+  }
+
   return json({
     path: bundlePath,
     manifest: {

From a2920cdb8b65749015a3c70279294042f4ebb7ce Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:13:21 -0500
Subject: [PATCH 14/21] fix(api): randomize uploaded .mcpb filename to prevent
 silent overwrite
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two uploads named `bundle.mcpb` previously clobbered each other on disk:
`safeBundleFilename` returned `basename(filename)` only, so the second
upload silently swapped the artifact backing whatever install was pinned
to that path in `workspace.json`.

Appends a 64-bit random hex suffix before the `.mcpb` extension so every
upload lands at a unique path. SHA-256 over the buffer was considered but
adds a 200 MB hash compute for no real benefit — idempotent re-upload is
not a requirement, and content-addressed storage would make collision
analysis a permanent maintenance burden.

The frontend never sees this name. Bundle display name comes from the
manifest via `extractBundleMeta` → `seedInstance` → `instance.bundleName`,
which `seedInstance` prefers over the filesystem label.

Updates `test/unit/mcpb-upload.test.ts`:
- Asserts the new `<stem>-<hex16>.mcpb` shape on existing traversal tests.
- Adds a uniqueness test pinning the no-clobber contract.
- Drops the `as unknown as { safeBundleFilename? }` cast — the function
  is a plain named export and the test file already imports from the
  same module.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/api/handlers.ts           | 27 +++++++++++++++-----
 test/unit/mcpb-upload.test.ts | 48 +++++++++++++++++------------------
 2 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index acb34e02..113487ae 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1046,15 +1046,28 @@ export function sanitizeFilename(name: string): string {
 /**
  * Resolve the safe on-disk filename for an uploaded `.mcpb` bundle.
  *
- * Strips every directory component via `path.basename`, so filenames like
- * `../../etc/cron.daily/evil.mcpb` collapse to `evil.mcpb` and cannot escape
- * the workspace bundles dir when joined onto it. `sanitizeFilename` only
- * neutralizes Content-Disposition-breaking chars and is insufficient on its
- * own — it would let `..` segments through and the upload would write
- * outside `bundlesDir`. Exported so tests pin the contract.
+ * Two responsibilities:
+ *
+ * 1. **Path traversal defense.** Strips every directory component via
+ *    `path.basename`, so filenames like `../../etc/cron.daily/evil.mcpb`
+ *    collapse to `evil.mcpb` and cannot escape the workspace bundles dir
+ *    when joined onto it. `sanitizeFilename` only neutralizes
+ *    Content-Disposition-breaking chars and is insufficient on its own.
+ *
+ * 2. **Collision avoidance.** Two uploads named `bundle.mcpb` would
+ *    otherwise clobber each other on disk, silently swapping the artifact
+ *    backing a running install (the path is the workspace.json key). A
+ *    64-bit random hex suffix is appended before the `.mcpb` extension so
+ *    every upload lands at a unique path. The frontend never sees this
+ *    name — bundle display name comes from the manifest, not the filename.
+ *
+ * Exported so tests pin the contract.
  */
 export function safeBundleFilename(filename: string): string {
-  return basename(filename);
+  const base = basename(filename);
+  const stem = base.endsWith(".mcpb") ? base.slice(0, -".mcpb".length) : base;
+  const suffix = randomBytes(8).toString("hex");
+  return `${stem}-${suffix}.mcpb`;
 }
 
 /**
diff --git a/test/unit/mcpb-upload.test.ts b/test/unit/mcpb-upload.test.ts
index 1a5b1fe0..8ccb5cd3 100644
--- a/test/unit/mcpb-upload.test.ts
+++ b/test/unit/mcpb-upload.test.ts
@@ -34,40 +34,38 @@ import { join } from "node:path";
 
 describe("Fix 1: path traversal in handleBundleUpload", () => {
 	it("safeBundleFilename strips traversal components", async () => {
-		const handlersModule = await import("../../src/api/handlers.ts");
-		const safeBundleFilename = (
-			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
-		).safeBundleFilename;
-
-		expect(typeof safeBundleFilename).toBe("function");
-		expect(safeBundleFilename!("../../etc/cron.daily/evil.mcpb")).toBe(
-			"evil.mcpb",
-		);
+		const { safeBundleFilename } = await import("../../src/api/handlers.ts");
+		const result = safeBundleFilename("../../etc/cron.daily/evil.mcpb");
+		// Random suffix appended before .mcpb to prevent collision; the
+		// stem is still derived from basename only, so traversal segments
+		// are stripped.
+		expect(result).toMatch(/^evil-[0-9a-f]{16}\.mcpb$/);
 	});
 
 	it("safeBundleFilename strips absolute path components", async () => {
-		const handlersModule = await import("../../src/api/handlers.ts");
-		const safeBundleFilename = (
-			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
-		).safeBundleFilename;
-
-		expect(typeof safeBundleFilename).toBe("function");
-		expect(safeBundleFilename!("/tmp/secrets/payload.mcpb")).toBe(
-			"payload.mcpb",
-		);
+		const { safeBundleFilename } = await import("../../src/api/handlers.ts");
+		const result = safeBundleFilename("/tmp/secrets/payload.mcpb");
+		expect(result).toMatch(/^payload-[0-9a-f]{16}\.mcpb$/);
 	});
 
 	it("joined path with safeBundleFilename stays inside bundlesDir", async () => {
-		const handlersModule = await import("../../src/api/handlers.ts");
-		const safeBundleFilename = (
-			handlersModule as unknown as { safeBundleFilename?: (s: string) => string }
-		).safeBundleFilename;
-
-		expect(typeof safeBundleFilename).toBe("function");
+		const { safeBundleFilename } = await import("../../src/api/handlers.ts");
 		const bundlesDir = "/home/.nimblebrain/workspaces/ws_dev/bundles";
-		const result = join(bundlesDir, safeBundleFilename!("../../evil.mcpb"));
+		const result = join(bundlesDir, safeBundleFilename("../../evil.mcpb"));
 		expect(result.startsWith(bundlesDir)).toBe(true);
 	});
+
+	it("safeBundleFilename returns a unique name on every call", async () => {
+		const { safeBundleFilename } = await import("../../src/api/handlers.ts");
+		// Two uploads with the same source filename must not produce the
+		// same on-disk path — otherwise the second clobbers the first
+		// silently and breaks any install pinned to that path.
+		const a = safeBundleFilename("bundle.mcpb");
+		const b = safeBundleFilename("bundle.mcpb");
+		expect(a).not.toBe(b);
+		expect(a).toMatch(/^bundle-[0-9a-f]{16}\.mcpb$/);
+		expect(b).toMatch(/^bundle-[0-9a-f]{16}\.mcpb$/);
+	});
 });
 
 // ---------------------------------------------------------------------------

From 47ffd53857c795aa5f0468f89811145eaf6c723e Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:14:03 -0500
Subject: [PATCH 15/21] fix(api): enforce MAX_BUNDLE_SIZE post-buffer in
 handleBundleUpload
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The route-level `bodyLimit(..., { multipart: MAX_BUNDLE_SIZE })` is
advisory — it inspects `Content-Length` only and lets chunked transfer
encoding, missing headers, or a lying client through. The handler then
buffered the upload into memory regardless of size.

Adds a post-buffer authoritative check on `data.length` immediately after
`arrayBuffer()` resolves, returning 413 with `{limit, received}`. Promotes
`MAX_BUNDLE_SIZE` to an exported constant in `handlers.ts` so the route
middleware and the handler share a single source of truth.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/api/handlers.ts       | 18 ++++++++++++++++++
 src/api/routes/bundles.ts |  4 +---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index 113487ae..2c469e36 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -1043,6 +1043,14 @@ export function sanitizeFilename(name: string): string {
   return name.replace(/["\r\n\x00-\x1f]/g, "_");
 }
 
+/**
+ * Maximum byte size of an uploaded `.mcpb` archive. Shared with the
+ * route-level `bodyLimit(..., { multipart: MAX_BUNDLE_SIZE })` so the
+ * `Content-Length`-advisory check and the post-buffer authoritative check
+ * stay in lockstep.
+ */
+export const MAX_BUNDLE_SIZE = 200 * 1024 * 1024; // 200 MB
+
 /**
  * Resolve the safe on-disk filename for an uploaded `.mcpb` bundle.
  *
@@ -1452,6 +1460,16 @@ export async function handleBundleUpload(
   if (data.length === 0) {
     return apiError(400, "bad_request", "Uploaded file is empty");
   }
+  // Authoritative size check. The route-level `bodyLimit` middleware is
+  // advisory: it only rejects when the client sends a `Content-Length`
+  // header. Chunked transfer encoding, missing headers, or a lying client
+  // bypass it — we only know the real size after buffering.
+  if (data.length > MAX_BUNDLE_SIZE) {
+    return apiError(413, "payload_too_large", "Bundle exceeds maximum size", {
+      limit: MAX_BUNDLE_SIZE,
+      received: data.length,
+    });
+  }
 
   // Validate-then-commit:
   //
diff --git a/src/api/routes/bundles.ts b/src/api/routes/bundles.ts
index 0b60aadf..d03353c3 100644
--- a/src/api/routes/bundles.ts
+++ b/src/api/routes/bundles.ts
@@ -1,13 +1,11 @@
 import { Hono } from "hono";
-import { handleBundleUpload } from "../handlers.ts";
+import { handleBundleUpload, MAX_BUNDLE_SIZE } from "../handlers.ts";
 import { requireAuth } from "../middleware/auth.ts";
 import { bodyLimit } from "../middleware/body-limit.ts";
 import { errorLog } from "../middleware/error-log.ts";
 import { requireWorkspace } from "../middleware/workspace.ts";
 import type { AppContext, AppEnv } from "../types.ts";
 
-const MAX_BUNDLE_SIZE = 200 * 1024 * 1024; // 200 MB
-
 export function bundleRoutes(ctx: AppContext) {
   return new Hono<AppEnv>()
     .use("*", requireAuth(ctx.authOptions))

From 530c241b407cc9ba281df9f13c043139beb38a57 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:15:26 -0500
Subject: [PATCH 16/21] refactor(api): extract UploadedFileEntry helper, adopt
 in upload handlers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three multipart handlers (`handleChatMultipart`, `handleResourceUpload`,
`handleBundleUpload`) each carried an inline
`value as unknown as { arrayBuffer(): Promise<ArrayBuffer>; ... }` cast
plus a duck-typed `arrayBuffer` check. The cast exists because Bun's
`Request.formData()` returns the undici `FormData` whose entries don't
unify with the DOM-lib `File` type at annotation sites.

Centralises both pieces in `src/api/types.ts`:

- `UploadedFileEntry` interface — minimal `arrayBuffer` / `name` / `type` /
  `size` shape that all callers share.
- `asUploadedFile(value)` helper — narrows a FormData value to the
  interface or returns `null`. Single source for the cast.

Three handlers now collapse to `const entry = asUploadedFile(value); if
(!entry) ...`. Adds ~30 LOC of shared types, drops ~20 LOC of inline
casts and duck-typing per site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/api/handlers.ts | 37 +++++++++----------------------------
 src/api/types.ts    | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/api/handlers.ts b/src/api/handlers.ts
index 2c469e36..eda5bb5b 100644
--- a/src/api/handlers.ts
+++ b/src/api/handlers.ts
@@ -26,7 +26,7 @@ import type { SseEventManager } from "./events.ts";
 import { ChatRequestBody, ToolCallRequestEnvelope } from "./schemas/rest.ts";
 import { validateAgainst } from "./schemas/validate.ts";
 import { startSseHeartbeat } from "./sse-heartbeat.ts";
-import { apiError } from "./types.ts";
+import { apiError, asUploadedFile } from "./types.ts";
 
 const pkgPath = resolve(import.meta.dirname ?? __dirname, "../../package.json");
 const pkg = JSON.parse(readFileSync(pkgPath, "utf-8")) as { version: string };
@@ -1198,17 +1198,12 @@ async function parseMultipartChatBody(
     }
   }
 
-  // Collect uploaded files — FormDataEntryValue is string | File in Bun.
-  // TypeScript without DOM lib doesn't know File, so we check via duck typing.
+  // Collect uploaded files — see `asUploadedFile` for why we don't annotate
+  // entries as `File` directly (Bun/undici vs DOM-lib type mismatch).
   const uploadedFiles: UploadedFile[] = [];
   for (const [_key, value] of formData.entries()) {
-    if (typeof value === "string") continue;
-    const entry = value as unknown as {
-      arrayBuffer(): Promise<ArrayBuffer>;
-      name?: string;
-      type?: string;
-    };
-    if (typeof entry.arrayBuffer !== "function") continue;
+    const entry = asUploadedFile(value);
+    if (!entry) continue;
     const buffer = Buffer.from(await entry.arrayBuffer());
     uploadedFiles.push({
       data: buffer,
@@ -1317,14 +1312,9 @@ export async function handleResourceUpload(
   const uploads: UploadedFile[] = [];
   try {
     for (const [key, value] of formData.entries()) {
-      if (typeof value === "string") continue;
       if (key !== "file" && key !== "files") continue;
-      const entry = value as unknown as {
-        arrayBuffer(): Promise<ArrayBuffer>;
-        name?: string;
-        type?: string;
-      };
-      if (typeof entry.arrayBuffer !== "function") continue;
+      const entry = asUploadedFile(value);
+      if (!entry) continue;
       uploads.push({
         data: Buffer.from(await entry.arrayBuffer()),
         filename: entry.name || "unnamed",
@@ -1433,8 +1423,8 @@ export async function handleBundleUpload(
     return apiError(400, "bad_request", "Expected multipart/form-data");
   }
 
-  const fileEntry = formData.get("file") ?? formData.get("bundle");
-  if (!fileEntry || typeof fileEntry === "string") {
+  const entry = asUploadedFile(formData.get("file") ?? formData.get("bundle"));
+  if (!entry) {
     return apiError(
       400,
       "bad_request",
@@ -1442,15 +1432,6 @@ export async function handleBundleUpload(
     );
   }
 
-  const entry = fileEntry as unknown as {
-    arrayBuffer(): Promise<ArrayBuffer>;
-    name?: string;
-    type?: string;
-  };
-  if (typeof entry.arrayBuffer !== "function") {
-    return apiError(400, "bad_request", "Malformed file entry in multipart body");
-  }
-
   const filename = entry.name || "bundle.mcpb";
   if (!filename.endsWith(".mcpb")) {
     return apiError(400, "bad_request", "File must have .mcpb extension");
diff --git a/src/api/types.ts b/src/api/types.ts
index b4c3c045..c3af5259 100644
--- a/src/api/types.ts
+++ b/src/api/types.ts
@@ -11,6 +11,39 @@ import type { SseEventManager } from "./events.ts";
 import type { McpServerHost } from "./mcp-server.ts";
 import type { LoginRateLimiter, RequestRateLimiter } from "./rate-limiter.ts";
 
+// ---------------------------------------------------------------------------
+// Multipart upload helper
+// ---------------------------------------------------------------------------
+
+/**
+ * Minimal interface for an uploaded file pulled out of `Request.formData()`.
+ *
+ * Why this exists: Bun's `Request.formData()` returns the undici `FormData`,
+ * whose entries are undici `File` instances. The DOM-lib `File` resolved at
+ * an annotation site has incompatible iterator types, so a direct
+ * `value as File` cast produces a TS error. Every multipart handler had its
+ * own `value as unknown as { arrayBuffer(): Promise<ArrayBuffer>; ... }`
+ * cast inline; this interface centralizes the shape.
+ */
+export interface UploadedFileEntry {
+  arrayBuffer(): Promise<ArrayBuffer>;
+  name?: string;
+  type?: string;
+  size?: number;
+}
+
+/**
+ * Narrow a `FormData.get` / `FormData.entries` value to an
+ * `UploadedFileEntry`. Returns `null` for strings, missing values, or
+ * objects without an `arrayBuffer` method (which would fail at read time
+ * anyway). Centralises the cross-type cast so handlers don't repeat it.
+ */
+export function asUploadedFile(value: unknown): UploadedFileEntry | null {
+  if (!value || typeof value === "string") return null;
+  const entry = value as UploadedFileEntry;
+  return typeof entry.arrayBuffer === "function" ? entry : null;
+}
+
 // ---------------------------------------------------------------------------
 // Standardized API error response
 // ---------------------------------------------------------------------------

From b215974f5cec50bd1ae86ca13a7fecf45e5e9c7d Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:16:14 -0500
Subject: [PATCH 17/21] feat(web): track upload phase explicitly in AboutTab
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previous shape used a single `uploadStatus: string | null` that doubled as
both phase indicator and error message. Disable state and button label
were derived via string-equality checks against the literal copy
("Validating..." / "Installing..."), which would silently break if the
strings were ever changed.

Splits state in two:

- `uploadPhase: "idle" | "validating" | "installing"` — drives the button
  label and disabled state. `isUploading` derived from `phase !== "idle"`.
- `uploadError: string | null` — error surfaced in the inline panel.
  Independent so a phase transition doesn't have to clear an error string.

Wraps the phase reset in `finally` so a thrown error from `uploadBundle`
or `callTool` always returns the button to the idle state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 web/src/pages/settings/AboutTab.tsx | 48 ++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/web/src/pages/settings/AboutTab.tsx b/web/src/pages/settings/AboutTab.tsx
index eb536c49..e1b66ff5 100644
--- a/web/src/pages/settings/AboutTab.tsx
+++ b/web/src/pages/settings/AboutTab.tsx
@@ -62,7 +62,23 @@ export function AboutTab() {
   const [apps, setApps] = useState<AppInfo[]>([]);
   const [loading, setLoading] = useState(true);
   const [bundlesError, setBundlesError] = useState<string | null>(null);
-  const [uploadStatus, setUploadStatus] = useState<string | null>(null);
+  // Two pieces of upload state, separated:
+  //
+  //   uploadPhase  — what the upload is doing right now ("validating" /
+  //                  "installing" / "idle"). Drives the button label and
+  //                  the disabled state. `isUploading` is derived from it.
+  //   uploadError  — error message to surface in the inline-error panel.
+  //                  Independent so we can show "validation failed" without
+  //                  string-equality-checking the phase.
+  //
+  // Previous shape used a single `uploadStatus: string | null` that doubled
+  // as both phase indicator and error message, requiring string-equality
+  // checks ("Validating..." / "Installing...") to decide whether the button
+  // should be disabled. That coupled the UI to specific status copy and
+  // would silently break if the strings were ever changed.
+  const [uploadPhase, setUploadPhase] = useState<"idle" | "validating" | "installing">("idle");
+  const [uploadError, setUploadError] = useState<string | null>(null);
+  const isUploading = uploadPhase !== "idle";
   const fileRef = useRef<HTMLInputElement>(null);
 
   const fetchApps = useCallback(async () => {
@@ -87,13 +103,14 @@ export function AboutTab() {
   const handleUpload = useCallback(
     async (file: File) => {
       if (!file.name.endsWith(".mcpb")) {
-        setUploadStatus("File must have .mcpb extension");
+        setUploadError("File must have .mcpb extension");
         return;
       }
-      setUploadStatus("Validating...");
+      setUploadError(null);
+      setUploadPhase("validating");
       try {
         const result = await uploadBundle(file);
-        setUploadStatus("Installing...");
+        setUploadPhase("installing");
         const installResult = await callTool("nb", "manage_app", {
           action: "install",
           path: result.path,
@@ -104,14 +121,15 @@ export function AboutTab() {
               ?.filter((c: { type: string }) => c.type === "text")
               .map((c: { text: string }) => c.text)
               .join("") ?? "Install failed";
-          setUploadStatus(msg);
+          setUploadError(msg);
           return;
         }
-        setUploadStatus(null);
         setLoading(true);
         fetchApps();
       } catch (err: unknown) {
-        setUploadStatus(err instanceof Error ? err.message : String(err));
+        setUploadError(err instanceof Error ? err.message : String(err));
+      } finally {
+        setUploadPhase("idle");
       }
     },
     [fetchApps],
@@ -150,12 +168,14 @@ export function AboutTab() {
               variant="outline"
               size="sm"
               onClick={() => fileRef.current?.click()}
-              disabled={uploadStatus === "Validating..." || uploadStatus === "Installing..."}
+              disabled={isUploading}
             >
               <Upload className="mr-1 h-3.5 w-3.5" />
-              {uploadStatus === "Validating..." || uploadStatus === "Installing..."
-                ? uploadStatus
-                : "Upload"}
+              {uploadPhase === "validating"
+                ? "Validating..."
+                : uploadPhase === "installing"
+                  ? "Installing..."
+                  : "Upload"}
             </Button>
           </>
         }
@@ -170,11 +190,11 @@ export function AboutTab() {
           </Link>
           .
         </p>
-        {uploadStatus && uploadStatus !== "Validating..." && uploadStatus !== "Installing..." ? (
+        {uploadError ? (
           <InlineError
-            message={uploadStatus}
+            message={uploadError}
             action={
-              <Button variant="outline" size="sm" onClick={() => setUploadStatus(null)}>
+              <Button variant="outline" size="sm" onClick={() => setUploadError(null)}>
                 Dismiss
               </Button>
             }

From ed5f8c407e5d9021da04e0e2b1caad4e46d5a601 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:17:55 -0500
Subject: [PATCH 18/21] test: import production filter in mcpb-upload unit test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Fix 3b unit test previously re-implemented the workspace.json filter
locally as `fixedFilter` — drift between the test copy and the production
filter would not have been caught.

Extracts the dispatch into a named export `bundleEntryMatchesTarget` in
`src/tools/system-tools.ts` and adopts it at both call sites:

- Install-side `bundles.some(...)` duplicate guard.
- Uninstall-side `bundles.filter(...)` removal pass.

Test imports it directly. Local helper is now a thin wrapper around
`!bundleEntryMatchesTarget(...)` — kept only so each test reads as
"remove the matching entry" rather than threading the negation through
every assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/tools/system-tools.ts     | 32 +++++++++++++++++++++++++++++++
 test/unit/mcpb-upload.test.ts | 36 ++++++++++++++++-------------------
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/src/tools/system-tools.ts b/src/tools/system-tools.ts
index 9f8b53e8..7bb5a2a7 100644
--- a/src/tools/system-tools.ts
+++ b/src/tools/system-tools.ts
@@ -50,6 +50,38 @@ export interface ManageBundleContext {
 /** Callback that returns the current loaded skills from the runtime. */
 export type GetSkillsFn = () => { context: Skill[]; matchable: Skill[] };
 
+/**
+ * Target identifier used by the manage_app install/uninstall flow — exactly
+ * one of `name` / `path` is expected to be set at the call site.
+ */
+export interface BundleTarget {
+  name?: string;
+  path?: string;
+}
+
+/**
+ * Decide whether a `workspace.json` bundle entry matches the install /
+ * uninstall target. Each variant matches by its own discriminant — a
+ * `{ name }` target matches `{ name }` entries on string equality;
+ * a `{ path }` target matches `{ path }` entries on string equality.
+ *
+ * Used by both the install-side duplicate guard (`bundles.some`) and the
+ * uninstall-side removal filter (`bundles.filter(b => !match(b, t))`). The
+ * previous uninstall filter only inspected `"name" in b` and so left
+ * `{ path }` entries stranded in `workspace.json` after their tool source
+ * had been deregistered.
+ *
+ * Exported so unit tests pin the contract without re-implementing it.
+ */
+export function bundleEntryMatchesTarget(
+  entry: import("../bundles/types.ts").BundleRef,
+  target: BundleTarget,
+): boolean {
+  if (target.name && "name" in entry) return entry.name === target.name;
+  if (target.path && "path" in entry) return entry.path === target.path;
+  return false;
+}
+
 /**
  * Factory that creates the `nb` system source as an in-process MCP server.
  * Merges core platform tools (list_apps, get_config, etc.) with system tools
diff --git a/test/unit/mcpb-upload.test.ts b/test/unit/mcpb-upload.test.ts
index 8ccb5cd3..aea4587b 100644
--- a/test/unit/mcpb-upload.test.ts
+++ b/test/unit/mcpb-upload.test.ts
@@ -19,6 +19,8 @@
 
 import { describe, expect, it } from "bun:test";
 import { join } from "node:path";
+import type { BundleRef } from "../../src/bundles/types.ts";
+import { bundleEntryMatchesTarget } from "../../src/tools/system-tools.ts";
 
 // ---------------------------------------------------------------------------
 // Fix 1: Path traversal — uploaded filename must strip directory components
@@ -74,34 +76,28 @@ describe("Fix 1: path traversal in handleBundleUpload", () => {
 // The previous filter took a single `target: string` and only matched
 // `{name}` entries — path-installed bundles became permanent residents of
 // workspace.json even after their tool source was deregistered. The new
-// filter takes `{name?, path?}` and dispatches per-variant.
-//
-// This is a unit test of the filter contract. Production wiring lives in
-// uninstallBundleFromWorkspaceViaCtx (system-tools.ts) and is exercised by
-// manual testing today; integration coverage tracks with the deferred Fix
-// 4/5/6 tests above.
+// dispatch lives in `bundleEntryMatchesTarget` (system-tools.ts) and is
+// shared between the install-side duplicate guard and the uninstall-side
+// removal filter. We import it directly so the test pins the production
+// contract rather than re-implementing it.
 // ---------------------------------------------------------------------------
 
 describe("Fix 3b: uninstall workspace.json filter handles {path} entries", () => {
-	function fixedFilter(
-		bundles: Array<{ name?: string; path?: string }>,
+	function removeMatching(
+		bundles: BundleRef[],
 		target: { name?: string; path?: string },
-	): Array<{ name?: string; path?: string }> {
-		return bundles.filter((b) => {
-			if (target.name && "name" in b) return b.name !== target.name;
-			if (target.path && "path" in b) return b.path !== target.path;
-			return true;
-		});
+	): BundleRef[] {
+		return bundles.filter((b) => !bundleEntryMatchesTarget(b, target));
 	}
 
 	it("removing a path-based bundle removes the {path} entry", () => {
-		const bundles = [
+		const bundles: BundleRef[] = [
 			{ name: "@acme/hello" },
 			{ path: "/uploads/custom.mcpb" },
 			{ name: "@acme/world" },
 		];
 
-		const result = fixedFilter(bundles, { path: "/uploads/custom.mcpb" });
+		const result = removeMatching(bundles, { path: "/uploads/custom.mcpb" });
 
 		expect(result).toHaveLength(2);
 		expect(
@@ -110,13 +106,13 @@ describe("Fix 3b: uninstall workspace.json filter handles {path} entries", () =>
 	});
 
 	it("removing a named bundle still works", () => {
-		const bundles = [
+		const bundles: BundleRef[] = [
 			{ name: "@acme/hello" },
 			{ path: "/uploads/custom.mcpb" },
 			{ name: "@acme/world" },
 		];
 
-		const result = fixedFilter(bundles, { name: "@acme/hello" });
+		const result = removeMatching(bundles, { name: "@acme/hello" });
 
 		expect(result).toHaveLength(2);
 		expect(
@@ -125,12 +121,12 @@ describe("Fix 3b: uninstall workspace.json filter handles {path} entries", () =>
 	});
 
 	it("name-targeted filter does not accidentally match path entries", () => {
-		const bundles = [
+		const bundles: BundleRef[] = [
 			{ path: "/uploads/echo.mcpb" },
 			{ name: "/uploads/echo.mcpb" },
 		];
 
-		const result = fixedFilter(bundles, { name: "/uploads/echo.mcpb" });
+		const result = removeMatching(bundles, { name: "/uploads/echo.mcpb" });
 
 		expect(result).toHaveLength(1);
 		expect(result[0]).toEqual({ path: "/uploads/echo.mcpb" });

From 665358aa3f16c271d6fccaa6f62b672f302b1ae1 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:22:25 -0500
Subject: [PATCH 19/21] test: cover .mcpb path guard and upload integration
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two test files address the coverage gap flagged in the PR review.

`test/unit/bundle-path-guard.test.ts` — pure-logic tests for
`assertPathInWorkspaceBundlesDir` (Critical #1 guard). Covers:

- Files inside the bundles dir pass.
- Non-existent files inside the dir pass (uninstall after manual delete).
- Absolute paths outside the dir reject.
- Traversal paths that resolve outside the dir reject.
- Cross-tenant: another workspace's bundles dir rejects.
- Symlinks inside bundlesDir that point outside reject (realpath defense).

The macOS-specific `/var` → `/private/var` symlink quirk surfaced a real
divergence between `realpathSync(existingDir)` and lexical `resolve(missing)`
on the same logical path. Adds a `canonicalize()` helper in `paths.ts` that
walks up to the deepest existing ancestor and realpaths from there, so the
guard agrees with itself regardless of which side is missing.

`test/integration/mcpb-bundle-upload.test.ts` — end-to-end coverage of the
upload flow (Fix 4 / Fix 5 / Fix 6 surface). Builds a real `.mcpb` archive
via the system `zip` CLI (manifest.json + a stub `server.sh` so validation
passes) and drives `POST /v1/bundles/upload`:

- Valid upload → 200, manifest echoed back, file inside bundles dir.
- Two uploads of the same source name → distinct on-disk paths
  (no-clobber contract from the random-suffix change).
- `../../etc/cron.daily/evil.mcpb` filename → traversal stripped.
- Non-`.mcpb` extension → 400, no disk side effects.
- Garbage archive → 400 + bundles dir empty (validate-then-rename).

Will fail until `@nimblebrain/mpak-sdk@>=0.7.0` is published with
`validateMcpb` exported and the dependency is bumped past 0.6.0.
Intentional — the failing test gates the SDK bump on the contract being
preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/bundles/paths.ts                        |  33 ++-
 test/integration/mcpb-bundle-upload.test.ts | 232 ++++++++++++++++++++
 test/unit/bundle-path-guard.test.ts         |  86 ++++++++
 3 files changed, 349 insertions(+), 2 deletions(-)
 create mode 100644 test/integration/mcpb-bundle-upload.test.ts
 create mode 100644 test/unit/bundle-path-guard.test.ts

diff --git a/src/bundles/paths.ts b/src/bundles/paths.ts
index 5a04fa56..27071f55 100644
--- a/src/bundles/paths.ts
+++ b/src/bundles/paths.ts
@@ -1,5 +1,5 @@
 import { existsSync, realpathSync } from "node:fs";
-import { join, resolve, sep } from "node:path";
+import { dirname, join, resolve, sep } from "node:path";
 import type { BundleRef } from "./types.ts";
 
 /** Prefixes reserved for system tools — bundles must not use these as source names. */
@@ -112,6 +112,30 @@ export function resolveWorkspaceBundlesDir(workDir: string, wsId: string): strin
   return join(workDir, "workspaces", wsId, "bundles");
 }
 
+/**
+ * Canonicalize a path for prefix comparison. Realpaths the file when it
+ * exists; when it doesn't, walks up the parent chain to find the deepest
+ * existing ancestor, realpaths that, then re-attaches the missing tail.
+ *
+ * The naive `existsSync(p) ? realpathSync(p) : resolve(p)` form diverges on
+ * platforms where ancestors are symlinks (macOS `/var` → `/private/var`):
+ * the bundles dir realpaths to the canonical form, the missing file does
+ * not, and a guard comparing the two falsely rejects.
+ */
+function canonicalize(filePath: string): string {
+  const abs = resolve(filePath);
+  if (existsSync(abs)) return realpathSync(abs);
+  // Walk up to find the deepest existing ancestor.
+  let parent = dirname(abs);
+  let suffix = abs.slice(parent.length); // includes leading sep
+  while (parent !== dirname(parent) && !existsSync(parent)) {
+    suffix = parent.slice(dirname(parent).length) + suffix;
+    parent = dirname(parent);
+  }
+  if (!existsSync(parent)) return abs;
+  return realpathSync(parent) + suffix;
+}
+
 /**
  * Assert that `filePath` resolves inside `<workDir>/workspaces/<wsId>/bundles/`.
  * Throws otherwise.
@@ -139,7 +163,12 @@ export function assertPathInWorkspaceBundlesDir(
   const canonicalBundlesDir = existsSync(bundlesDir)
     ? realpathSync(bundlesDir)
     : resolve(bundlesDir);
-  const canonicalFile = existsSync(filePath) ? realpathSync(filePath) : resolve(filePath);
+  // For a missing file, realpath the parent dir and re-attach the basename.
+  // Pure lexical resolve diverges from the bundles-dir canonicalization on
+  // platforms where ancestors are themselves symlinks (notably macOS, where
+  // /var → /private/var). Without this, an uninstall after manual deletion
+  // of a perfectly-valid bundle inside the dir would falsely fail the guard.
+  const canonicalFile = canonicalize(filePath);
   if (
     canonicalFile !== canonicalBundlesDir &&
     !canonicalFile.startsWith(canonicalBundlesDir + sep)
diff --git a/test/integration/mcpb-bundle-upload.test.ts b/test/integration/mcpb-bundle-upload.test.ts
new file mode 100644
index 00000000..5e38cae8
--- /dev/null
+++ b/test/integration/mcpb-bundle-upload.test.ts
@@ -0,0 +1,232 @@
+/**
+ * Integration coverage for the `.mcpb` upload + install path.
+ *
+ * Builds a real `.mcpb` archive on disk (zip of `manifest.json`) and drives
+ * the production code paths end-to-end:
+ *
+ * - `POST /v1/bundles/upload` — multipart upload, validation via mpak SDK,
+ *   tempfile-then-rename flow, manifest echo back.
+ * - Filename uniqueness — two uploads of the same source filename must land
+ *   at different on-disk paths.
+ * - Path-traversal filename — `../../etc/evil.mcpb` collapses to a basename
+ *   inside the workspace bundles dir.
+ * - Invalid archive — garbage bytes are rejected and leave nothing behind.
+ *
+ * **Status until mpak#94 ships.** These tests depend on `validateMcpb`
+ * being exported from `@nimblebrain/mpak-sdk`. The pinned version (0.6.0)
+ * does not yet export it, so `bun run test:integration` will fail this
+ * file until `@nimblebrain/mpak-sdk@>=0.7.0` is published and the
+ * dependency is bumped. That is intentional — landing the tests now keeps
+ * the contract pinned and turns the SDK bump into a one-line PR that the
+ * existing tests gate.
+ *
+ * Subprocess spawning (the full `manage_app({ action: "install", path })`
+ * round-trip) is intentionally NOT exercised here — bundle servers need
+ * their own runtime (Python, Node, etc.) and would make this file flaky.
+ * The smoke suite is the right home for end-to-end spawn coverage.
+ */
+
+import { afterAll, beforeAll, describe, expect, it } from "bun:test";
+import { existsSync, mkdirSync, readdirSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { type ServerHandle, startServer } from "../../src/api/server.ts";
+import { Runtime } from "../../src/runtime/runtime.ts";
+import { createEchoModel } from "../helpers/echo-model.ts";
+import { TEST_WORKSPACE_ID, provisionTestWorkspace } from "../helpers/test-workspace.ts";
+
+let runtime: Runtime;
+let handle: ServerHandle;
+let baseUrl: string;
+const testDir = join(tmpdir(), `nimblebrain-mcpb-upload-${Date.now()}`);
+const fixtureDir = join(testDir, "fixtures");
+
+/**
+ * Build a minimal valid `.mcpb` archive for a fake bundle named `bundleName`.
+ * `.mcpb` is just a zip of `manifest.json` (+ optional server payload).
+ * Uses the system `zip` CLI — available on every CI image we run on.
+ */
+async function buildFixture(bundleName: string, label: string): Promise<string> {
+  const stagingDir = join(fixtureDir, `staging-${label}`);
+  mkdirSync(stagingDir, { recursive: true });
+  // Validator requires the manifest's `entry_point` to exist inside the
+  // archive — include a stub script so validation passes. The script is
+  // never executed by these tests; we don't drive `manage_app({ install })`.
+  const stubScript = "#!/bin/sh\nexit 0\n";
+  writeFileSync(join(stagingDir, "server.sh"), stubScript, { mode: 0o755 });
+  const manifest = {
+    manifest_version: "0.4",
+    name: bundleName,
+    version: "0.0.1",
+    description: `Test fixture: ${bundleName}`,
+    author: { name: "test" },
+    server: {
+      type: "binary",
+      entry_point: "server.sh",
+      mcp_config: {
+        command: "${__dirname}/server.sh",
+        args: [],
+      },
+    },
+  };
+  writeFileSync(join(stagingDir, "manifest.json"), JSON.stringify(manifest));
+  const archivePath = join(fixtureDir, `${label}.mcpb`);
+  const proc = Bun.spawn(
+    [
+      "zip",
+      "-j",
+      archivePath,
+      join(stagingDir, "manifest.json"),
+      join(stagingDir, "server.sh"),
+    ],
+    { stdout: "ignore", stderr: "pipe" },
+  );
+  const exitCode = await proc.exited;
+  if (exitCode !== 0) {
+    const err = await new Response(proc.stderr).text();
+    throw new Error(`zip failed (${exitCode}): ${err}`);
+  }
+  return archivePath;
+}
+
+beforeAll(async () => {
+  mkdirSync(fixtureDir, { recursive: true });
+  runtime = await Runtime.start({
+    model: { provider: "custom", adapter: createEchoModel() },
+    noDefaultBundles: true,
+    logging: { disabled: true },
+    workDir: testDir,
+  });
+  await provisionTestWorkspace(runtime);
+  handle = startServer({ runtime, port: 0 });
+  baseUrl = `http://localhost:${handle.port}`;
+});
+
+afterAll(async () => {
+  handle.stop(true);
+  await runtime.shutdown();
+  rmSync(testDir, { recursive: true, force: true });
+});
+
+const bundlesDir = () => join(testDir, "workspaces", TEST_WORKSPACE_ID, "bundles");
+
+describe("POST /v1/bundles/upload", () => {
+  it("accepts a valid .mcpb, validates, and writes into the workspace bundles dir", async () => {
+    const archive = await buildFixture("@nb-test/echo", "echo-1");
+    const bytes = await Bun.file(archive).arrayBuffer();
+
+    const form = new FormData();
+    form.append(
+      "file",
+      new Blob([bytes], { type: "application/zip" }),
+      "echo.mcpb",
+    );
+
+    const res = await fetch(`${baseUrl}/v1/bundles/upload`, {
+      method: "POST",
+      headers: { "X-Workspace-Id": TEST_WORKSPACE_ID },
+      body: form,
+    });
+
+    expect(res.status).toBe(200);
+    const body = await res.json();
+    expect(body.manifest.name).toBe("@nb-test/echo");
+    expect(body.manifest.version).toBe("0.0.1");
+    // Path landed inside the workspace bundles dir.
+    expect(body.path.startsWith(bundlesDir())).toBe(true);
+    expect(existsSync(body.path)).toBe(true);
+  });
+
+  it("uses a randomized filename so two uploads of the same source name don't collide", async () => {
+    const archive = await buildFixture("@nb-test/echo2", "echo-2");
+    const bytes = await Bun.file(archive).arrayBuffer();
+
+    const upload = async () => {
+      const form = new FormData();
+      form.append(
+        "file",
+        new Blob([bytes], { type: "application/zip" }),
+        "bundle.mcpb",
+      );
+      const res = await fetch(`${baseUrl}/v1/bundles/upload`, {
+        method: "POST",
+        headers: { "X-Workspace-Id": TEST_WORKSPACE_ID },
+        body: form,
+      });
+      expect(res.status).toBe(200);
+      return (await res.json()).path as string;
+    };
+
+    const a = await upload();
+    const b = await upload();
+    expect(a).not.toBe(b);
+    expect(existsSync(a)).toBe(true);
+    expect(existsSync(b)).toBe(true);
+  });
+
+  it("strips path traversal segments from the upload filename", async () => {
+    const archive = await buildFixture("@nb-test/echo3", "echo-3");
+    const bytes = await Bun.file(archive).arrayBuffer();
+
+    const form = new FormData();
+    form.append(
+      "file",
+      new Blob([bytes], { type: "application/zip" }),
+      "../../etc/cron.daily/evil.mcpb",
+    );
+
+    const res = await fetch(`${baseUrl}/v1/bundles/upload`, {
+      method: "POST",
+      headers: { "X-Workspace-Id": TEST_WORKSPACE_ID },
+      body: form,
+    });
+
+    expect(res.status).toBe(200);
+    const body = await res.json();
+    // Stem is `evil` (basename strips traversal); random suffix appended.
+    expect(body.path.startsWith(bundlesDir())).toBe(true);
+    expect(body.path).toMatch(/\/evil-[0-9a-f]{16}\.mcpb$/);
+  });
+
+  it("rejects a non-.mcpb upload before touching disk", async () => {
+    const before = existsSync(bundlesDir()) ? readdirSync(bundlesDir()).length : 0;
+
+    const form = new FormData();
+    form.append("file", new Blob(["hello"], { type: "text/plain" }), "notes.txt");
+
+    const res = await fetch(`${baseUrl}/v1/bundles/upload`, {
+      method: "POST",
+      headers: { "X-Workspace-Id": TEST_WORKSPACE_ID },
+      body: form,
+    });
+
+    expect(res.status).toBe(400);
+    const after = existsSync(bundlesDir()) ? readdirSync(bundlesDir()).length : 0;
+    expect(after).toBe(before);
+  });
+
+  it("rejects an invalid archive and leaves the bundles dir clean", async () => {
+    const before = existsSync(bundlesDir()) ? readdirSync(bundlesDir()).length : 0;
+
+    const form = new FormData();
+    form.append(
+      "file",
+      new Blob(["not a real zip"], { type: "application/zip" }),
+      "broken.mcpb",
+    );
+
+    const res = await fetch(`${baseUrl}/v1/bundles/upload`, {
+      method: "POST",
+      headers: { "X-Workspace-Id": TEST_WORKSPACE_ID },
+      body: form,
+    });
+
+    expect(res.status).toBe(400);
+    const body = await res.json();
+    expect(body.error).toBe("invalid_bundle");
+    // No artifact landed in the bundles dir — validation runs against a
+    // tempfile and the rename only fires on success.
+    const after = existsSync(bundlesDir()) ? readdirSync(bundlesDir()).length : 0;
+    expect(after).toBe(before);
+  });
+});
diff --git a/test/unit/bundle-path-guard.test.ts b/test/unit/bundle-path-guard.test.ts
new file mode 100644
index 00000000..05f2d529
--- /dev/null
+++ b/test/unit/bundle-path-guard.test.ts
@@ -0,0 +1,86 @@
+/**
+ * Unit tests for `assertPathInWorkspaceBundlesDir` — the guard that confines
+ * any LLM-supplied `.mcpb` path to a single workspace's bundles directory.
+ *
+ * Without this guard, prompt-injected `manage_app({ action: "install",
+ * path })` could spawn an arbitrary `.mcpb` archive under the platform user
+ * with workspace credentials and `NB_INTERNAL_TOKEN` (for protected refs)
+ * attached — see PR review of #170 Critical #1.
+ *
+ * Pure-logic tests: no Runtime, no HTTP server. Filesystem touched only to
+ * exercise the realpath / lexical fallback distinction and the symlink
+ * defense.
+ */
+
+import { afterAll, beforeAll, describe, expect, it } from "bun:test";
+import { mkdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { assertPathInWorkspaceBundlesDir } from "../../src/bundles/paths.ts";
+
+const workDir = join(tmpdir(), `nb-bundle-path-guard-${Date.now()}`);
+const wsId = "ws_test";
+const bundlesDir = join(workDir, "workspaces", wsId, "bundles");
+
+beforeAll(() => {
+  mkdirSync(bundlesDir, { recursive: true });
+});
+
+afterAll(() => {
+  rmSync(workDir, { recursive: true, force: true });
+});
+
+describe("assertPathInWorkspaceBundlesDir", () => {
+  it("accepts a file directly inside the workspace bundles dir", () => {
+    const ok = join(bundlesDir, "echo-aaaaaaaaaaaaaaaa.mcpb");
+    writeFileSync(ok, "fake");
+    expect(() => assertPathInWorkspaceBundlesDir(ok, workDir, wsId)).not.toThrow();
+  });
+
+  it("accepts a non-existent file with a path inside the dir (lexical fallback)", () => {
+    // Uninstall after manual file deletion is allowed — the persisted
+    // workspace.json path was vetted at install time.
+    const stillInside = join(bundlesDir, "deleted-already.mcpb");
+    expect(() => assertPathInWorkspaceBundlesDir(stillInside, workDir, wsId)).not.toThrow();
+  });
+
+  it("rejects an absolute path outside the bundles dir", () => {
+    expect(() =>
+      assertPathInWorkspaceBundlesDir("/tmp/elsewhere.mcpb", workDir, wsId),
+    ).toThrow(/must live inside the workspace bundles directory/);
+  });
+
+  it("rejects a traversal path that resolves outside the bundles dir", () => {
+    const traversal = join(bundlesDir, "..", "..", "elsewhere.mcpb");
+    expect(() => assertPathInWorkspaceBundlesDir(traversal, workDir, wsId)).toThrow(
+      /must live inside the workspace bundles directory/,
+    );
+  });
+
+  it("rejects another workspace's bundles dir", () => {
+    // Cross-tenant guard: a bundle uploaded to ws_a must not be installable
+    // from ws_b. Even if the LLM in ws_b is told a valid path, the guard
+    // bound to ws_b's dir refuses it.
+    const otherWs = join(workDir, "workspaces", "ws_other", "bundles");
+    mkdirSync(otherWs, { recursive: true });
+    const otherBundle = join(otherWs, "neighbor.mcpb");
+    writeFileSync(otherBundle, "fake");
+    expect(() => assertPathInWorkspaceBundlesDir(otherBundle, workDir, wsId)).toThrow(
+      /must live inside the workspace bundles directory/,
+    );
+  });
+
+  it("rejects a symlink inside bundlesDir that targets outside the dir", () => {
+    // Defense: an attacker who can write to bundlesDir (or a previous
+    // exploit who got there) cannot escape via a symlink. realpath collapses
+    // the link before the prefix check.
+    const outside = join(workDir, "not-bundles", "evil.mcpb");
+    mkdirSync(join(workDir, "not-bundles"), { recursive: true });
+    writeFileSync(outside, "fake");
+    const linkInside = join(bundlesDir, "evil-symlink.mcpb");
+    symlinkSync(outside, linkInside);
+    expect(() => assertPathInWorkspaceBundlesDir(linkInside, workDir, wsId)).toThrow(
+      /must live inside the workspace bundles directory/,
+    );
+  });
+});

From cfcba623a4e6f2a688545f2d07aaaec0bb605743 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Wed, 6 May 2026 11:22:58 -0500
Subject: [PATCH 20/21] docs: changelog entry for .mcpb upload feature

Adds an `### Added` bullet under Unreleased covering the upload endpoint,
the `manage_app({ path })` install path, the workspace-bundles-dir
confinement guard, and persistence behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 61ab1e12..a8d1508c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -35,6 +35,7 @@
 - Top-level `/profile` route. Identity isn't a setting; un-nested from `/settings/*`.
 - Org-admin gate on `set_model_config` — backend now refuses non-org-admin writes (was UI-only via RouteGuard). Distinguishes "no identity" (cron, automations) from "wrong role" so debug logs make non-user code paths obvious.
 - HTTP proxy primitive (`_meta["ai.nimblebrain/http-proxy"]`). Bundles can expose a loopback HTTP server (e.g. `astro preview`, Jupyter kernel) through the platform at `/v1/ws/<wsId>/apps/<bundle>/<mount>/*`. Loopback-only target, credentials and `Accept-Encoding` stripped on forward, `Set-Cookie`/CSP/X-Frame-Options stripped on response, per-workspace kill switch via `Workspace.allowHttpProxy`. Bundles get `NB_WORKSPACE_ID`, `NB_PROXY_PREFIX`, `NB_PUBLIC_ORIGIN` in their env at spawn ([docs](https://docs.nimblebrain.ai/apps/http-proxy/)).
+- Upload custom `.mcpb` bundles from Settings → About. Multipart upload to `POST /v1/bundles/upload` validates the archive via `@nimblebrain/mpak-sdk` in a tempfile, then commits to the workspace bundles dir. `manage_app` gains a `path` parameter for file-based install — confined to the workspace bundles dir at install, uninstall, and startup re-hydration so a prompt-injected agent cannot spawn arbitrary `.mcpb` artifacts under workspace credentials. `.mcpb` paths persist in `workspace.json` and survive restart ([#170](https://github.com/NimbleBrainInc/nimblebrain/pull/170)).
 
 ### Changed
 

From 1cd6c033e6a6e3a63f12ac2d841dc461bcc69a80 Mon Sep 17 00:00:00 2001
From: Mason <31372737+Ovaculos@users.noreply.github.com>
Date: Mon, 11 May 2026 11:45:45 -0500
Subject: [PATCH 21/21] fix(rebase): drop duplicate bundleEntryMatchesTarget,
 narrow AboutTab content type
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two leftovers from rebasing onto upstream/main after the Connectors UX
landed in #166/#186/#195 and the canonical-id slug refactor moved the
helpers around:

1. `bundleEntryMatchesTarget` had already been extracted upstream
   (system-tools.ts:76); my Fix 3 commit redeclared it lower in the
   same file, which biome flagged as `lint/suspicious/noRedeclare`.
   Removed the duplicate and kept the upstream definition — the call
   sites already reference the right binding by name.

2. `AboutTab.tsx`'s upload error-extraction filter narrowed to
   `{ type: string }` only, so the subsequent map produced
   `text: string | undefined` (web's content schema marks it optional)
   and failed `cd web && bunx tsc --noEmit`. Tightened the filter to a
   type predicate that asserts both `type === "text"` and
   `typeof text === "string"`, so the map call sees a strict
   `{ type: "text"; text: string }`.

Verified locally: bun run verify:static, test:unit (2593/0), test:web
(257/0), test:integration (480/0/12 skip).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/tools/system-tools.ts           | 21 ---------------------
 web/src/pages/settings/AboutTab.tsx |  8 +++++---
 2 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/src/tools/system-tools.ts b/src/tools/system-tools.ts
index 7bb5a2a7..18f4e561 100644
--- a/src/tools/system-tools.ts
+++ b/src/tools/system-tools.ts
@@ -968,27 +968,6 @@ async function installBundleInWorkspaceViaCtx(
  * predate `serverName`-on-ref persistence. Exported so the regression
  * is unit-testable independently of the full uninstall stack.
  */
-/**
- * True if a persisted workspace.json `BundleRef` entry matches the
- * `{name?, path?}` target shape used by the install/uninstall handlers.
- *
- * One predicate, two call sites — the install pre-check ("is this bundle
- * already registered in workspace.json?") and the uninstall filter ("which
- * entry should I drop?"). Keeping the dispatch in one place means the two
- * stay in lockstep; the previous filter only matched `{name}` entries and
- * silently left `{path}`-installed bundles in workspace.json forever.
- *
- * Exported so the unit test pins the contract.
- */
-export function bundleEntryMatchesTarget(
-  entry: { name?: string; path?: string },
-  target: { name?: string; path?: string },
-): boolean {
-  if (target.name && "name" in entry) return entry.name === target.name;
-  if (target.path && "path" in entry) return entry.path === target.path;
-  return false;
-}
-
 export function resolveBundleServerName(
   bundleName: string,
   ws: { bundles: Array<{ name?: string; serverName?: string }> } | null,
diff --git a/web/src/pages/settings/AboutTab.tsx b/web/src/pages/settings/AboutTab.tsx
index e1b66ff5..0db1f4ab 100644
--- a/web/src/pages/settings/AboutTab.tsx
+++ b/web/src/pages/settings/AboutTab.tsx
@@ -118,8 +118,11 @@ export function AboutTab() {
         if (installResult.isError) {
           const msg =
             installResult.content
-              ?.filter((c: { type: string }) => c.type === "text")
-              .map((c: { text: string }) => c.text)
+              ?.filter(
+                (c): c is { type: "text"; text: string } =>
+                  c.type === "text" && typeof c.text === "string",
+              )
+              .map((c) => c.text)
               .join("") ?? "Install failed";
           setUploadError(msg);
           return;
@@ -201,7 +204,6 @@ export function AboutTab() {
           />
         ) : null}
 
-
         {loading ? (
           <p className="text-sm text-muted-foreground">Loading...</p>
         ) : bundlesError ? (