fix(appkit): skip JSON body parsing for parameterized skipBodyParsing routes

packages/appkit/src/context/tests/service-context.test.ts

@@ -80,6 +80,8 @@ describe("ServiceContext", () => {
     vi.clearAllMocks();
     ServiceContext.reset();
     setupDatabricksEnv();
+    // Databricks-hosted shells export this; it would short-circuit SCIM lookup
+    delete process.env.DATABRICKS_WORKSPACE_ID;
     setupDefaultMocks();
   });
 

packages/appkit/src/plugins/files/plugin.ts

@@ -1183,6 +1183,22 @@ export class FilesPlugin extends Plugin implements ToolProvider {
       return;
     }
 
+    // A body parser that already drained the request would make us write an
+    // empty file and report success; fail loudly instead.
+    if (req.readableEnded) {
+      logger.error(
+        req,
+        "Upload body already consumed: volume=%s path=%s",
+        volumeKey,
+        path,
+      );
+      res.status(500).json({
+        error: "Upload body was already consumed before the upload handler.",
+        plugin: this.name,
+      });
+      return;
+    }
+
     logger.debug(req, "Upload started: volume=%s path=%s", volumeKey, path);
 
     const { mode, userCtx } = this._resolveAuthForRequest(req, volumeKey);

packages/appkit/src/plugins/files/tests/plugin.integration.test.ts

@@ -53,6 +53,8 @@ const MOCK_AUTH_HEADERS = {
 
 /** Volume key used in all integration tests. */
 const VOL = "files";
+/** Allow-all volume for tests that need a successful upload. */
+const UPLOAD_VOL = "uploads";
 
 describe("Files Plugin Integration", () => {
   let server: Server;
@@ -62,6 +64,7 @@ describe("Files Plugin Integration", () => {
   beforeAll(async () => {
     setupDatabricksEnv({
       DATABRICKS_VOLUME_FILES: "/Volumes/catalog/schema/vol",
+      DATABRICKS_VOLUME_UPLOADS: "/Volumes/catalog/schema/uploads",
     });
     ServiceContext.reset();
 
@@ -78,7 +81,9 @@ describe("Files Plugin Integration", () => {
           port: 0,
           host: "127.0.0.1",
         }),
-        files(),
+        files({
+          volumes: { [UPLOAD_VOL]: { policy: files.policy.allowAll() } },
+        }),
       ],
     });
 
@@ -89,6 +94,7 @@ describe("Files Plugin Integration", () => {
 
   afterAll(async () => {
     delete process.env.DATABRICKS_VOLUME_FILES;
+    delete process.env.DATABRICKS_VOLUME_UPLOADS;
     serviceContextMock?.restore();
     if (server) {
       await new Promise<void>((resolve, reject) => {
@@ -117,7 +123,7 @@ describe("Files Plugin Integration", () => {
 
       expect(response.status).toBe(200);
       const data = await response.json();
-      expect(data).toEqual({ volumes: [VOL] });
+      expect(data).toEqual({ volumes: [VOL, UPLOAD_VOL] });
     });
   });
 
@@ -638,6 +644,28 @@ describe("Files Plugin Integration", () => {
     });
   });
 
+  describe("Upload Body", () => {
+    test("POST upload with Content-Type application/json uploads the raw body", async () => {
+      let uploaded: string | undefined;
+      mockFilesApi.uploadFile.mockImplementation(async ({ contents }) => {
+        uploaded = await new Response(contents).text();
+      });
+      const body = JSON.stringify({ hello: "world" });
+
+      const response = await fetch(
+        `${baseUrl}/api/files/${UPLOAD_VOL}/upload?path=data.json`,
+        {
+          method: "POST",
+          headers: { ...MOCK_AUTH_HEADERS, "Content-Type": "application/json" },
+          body,
+        },
+      );
+
+      expect(response.status).toBe(200);
+      expect(uploaded).toBe(body);
+    });
+  });
+
   describe("Error Handling", () => {
     test("SDK exceptions return 500 with generic error", async () => {
       mockSdkClient.request.mockRejectedValue(

packages/appkit/src/plugins/files/tests/upload.test.ts

@@ -184,4 +184,31 @@ describe("FilesPlugin upload", () => {
       expect(del).toHaveBeenCalled();
     });
   });
+
+  describe("Upload body already consumed", () => {
+    test("returns 500 instead of uploading an empty file", async () => {
+      const plugin = new FilesPlugin(VOLUMES_CONFIG);
+      const handler = getRouteHandler(plugin, "post", "/upload");
+      const res = mockRes();
+
+      const req = mockUploadReq("uploads", [Buffer.from('{"a":1}')], {
+        query: { path: "/Volumes/catalog/schema/uploads/data.json" },
+      });
+      // Drain the body the way a body parser running first would
+      for await (const _ of req);
+
+      const connector = (plugin as any).volumeConnectors.uploads;
+      const upload = vi.spyOn(connector, "upload");
+
+      await handler(req, res);
+
+      expect(res.status).toHaveBeenCalledWith(500);
+      expect(res.json).toHaveBeenCalledWith(
+        expect.objectContaining({
+          error: expect.stringContaining("already consumed"),
+        }),
+      );
+      expect(upload).not.toHaveBeenCalled();
+    });
+  });
 });

packages/appkit/src/plugins/server/index.ts

@@ -77,7 +77,7 @@ export class ServerPlugin extends Plugin {
   private resolvedListenPort?: number;
   declare protected config: ServerConfig;
   private serverExtensions: ((app: express.Application) => void)[] = [];
-  private rawBodyPaths: Set<string> = new Set();
+  private rawBodyPaths: RegExp[] = [];
   /**
    * Resolves when `server.close()` completes. Created in
    * {@link abortActiveOperations} (which initiates the close) and awaited in
@@ -145,9 +145,10 @@ export class ServerPlugin extends Plugin {
           // Skip JSON parsing for routes that declared skipBodyParsing
           // (e.g. file uploads where the raw body must flow through).
           // rawBodyPaths is populated by extendRoutes() below; the type
-          // callback runs per-request so the set is already filled.
+          // callback runs per-request so the list is already filled.
           const urlPath = req.url?.split("?")[0];
-          if (urlPath && this.rawBodyPaths.has(urlPath)) return false;
+          if (urlPath && this.rawBodyPaths.some((re) => re.test(urlPath)))
+            return false;
           const ct = req.headers["content-type"] ?? "";
           return ct.includes("json");
         },
@@ -289,8 +290,17 @@ export class ServerPlugin extends Plugin {
           plugin.getSkipBodyParsingPaths &&
           typeof plugin.getSkipBodyParsingPaths === "function"
         ) {
+          // Paths are route patterns (e.g. `/api/files/:volumeKey/upload`).
+          // Match them like Express 4 does by default: one segment per
+          // `:param`, optional trailing slash, case-insensitive.
+          // shortcut: only `:param` segments are supported, not Express
+          // wildcards/regex paths; switch to path-to-regexp if a
+          // skipBodyParsing route needs them.
           for (const p of plugin.getSkipBodyParsingPaths()) {
-            this.rawBodyPaths.add(p);
+            const source = p
+              .replace(/[.*+?^${}()|[\]\\]/g, "\\$&")
+              .replace(/:\w+/g, "[^/]+");
+            this.rawBodyPaths.push(new RegExp(`^${source}/?$`, "i"));
           }
         }
       }

packages/appkit/src/plugins/server/tests/server.test.ts

@@ -415,40 +415,39 @@ describe("ServerPlugin", () => {
           getEndpoints: vi.fn().mockReturnValue({}),
           getSkipBodyParsingPaths: vi
             .fn()
-            .mockReturnValue(new Set(["/api/files/upload"])),
+            .mockReturnValue(new Set(["/api/files/:volumeKey/upload"])),
         },
       };
 
-      const plugin = new ServerPlugin({ plugins });
+      const plugin = new ServerPlugin({
+        context: createContextWithPlugins(plugins),
+      } as any);
       await plugin.start();
 
       // Get the type function passed to express.json
-      const jsonCall = vi.mocked(express.json).mock.calls[0][0] as any;
-      const typeFn = jsonCall.type;
-
-      // Should skip body parsing for the declared path
-      expect(typeFn({ url: "/api/files/upload", headers: {} })).toBe(false);
-
-      // Should skip body parsing for declared path with query string
-      expect(typeFn({ url: "/api/files/upload?path=foo", headers: {} })).toBe(
-        false,
-      );
-
-      // Should NOT skip body parsing for other routes (no hardcoded /upload check)
-      expect(
-        typeFn({
-          url: "/api/other/upload",
-          headers: { "content-type": "application/json" },
-        }),
-      ).toBe(true);
-
-      // Should still parse JSON for normal routes
-      expect(
-        typeFn({
-          url: "/api/analytics/query",
-          headers: { "content-type": "application/json" },
-        }),
-      ).toBe(true);
+      const typeFn = (vi.mocked(express.json).mock.calls[0][0] as any).type;
+      const json = { "content-type": "application/json" };
+
+      // Every URL Express routes to the declared pattern skips parsing
+      for (const url of [
+        "/api/files/files/upload",
+        "/api/files/files/upload?path=data.json",
+        "/api/files/files/upload/",
+        "/API/Files/files/Upload",
+      ]) {
+        expect(typeFn({ url, headers: json })).toBe(false);
+      }
+
+      // Other routes still parse JSON; `:volumeKey` matches exactly one segment
+      for (const url of [
+        "/api/files/files/mkdir",
+        "/api/files/a/b/upload",
+        "/api/files//upload",
+        "/api/other/upload",
+        "/api/analytics/query",
+      ]) {
+        expect(typeFn({ url, headers: json })).toBe(true);
+      }
     });
 
     test("extendRoutes registers plugin routes correctly", async () => {

vitest.config.ts

@@ -4,6 +4,11 @@ import react from "@vitejs/plugin-react";
 import tsconfigPaths from "vite-tsconfig-paths";
 import { defineConfig } from "vitest/config";
 
+// Some suites import large module graphs inside tests and hooks (lazy
+// `await import`), so under CPU load with coverage the 5s/10s defaults time
+// out on module transforms rather than on the code under test.
+const slowImportTimeouts = { testTimeout: 30_000, hookTimeout: 30_000 };
+
 export default defineConfig({
   test: {
     globals: true,
@@ -28,6 +33,7 @@ export default defineConfig({
           name: "playground",
           root: "./apps/dev-playground",
           environment: "node",
+          ...slowImportTimeouts,
           // Playwright owns tests/*.spec.ts; these are isolated server unit tests.
           include: ["server/**/*.test.ts"],
         },
@@ -59,6 +65,7 @@ export default defineConfig({
           name: "appkit",
           root: "./packages/appkit",
           environment: "node",
+          ...slowImportTimeouts,
         },
       },
       {
@@ -83,6 +90,7 @@ export default defineConfig({
           name: "dev-playground",
           root: "./apps/dev-playground",
           environment: "node",
+          ...slowImportTimeouts,
           // tests/ holds Playwright specs. Vitest's default `**/*.spec.ts`
           // glob would collect them and they fail on import with "Playwright
           // Test did not expect test.describe() to be called here". They run