Fix the local e2e suite and guard it in CI

.changeset/local-toolkit-mcp-db-lock.md

@@ -0,0 +1,7 @@
+---
+"executor": patch
+---
+
+**Fix: toolkit-scoped MCP endpoints on the local server no longer fail with an internal error**
+
+`POST /mcp/toolkits/<slug>` returned `-32603 Internal server error` for every request. Building a toolkit-scoped session called `createExecutorHandle`, which opened the local data directory a second time — but the running server already holds that directory's exclusive ownership lock, so the open failed against the server's own lock ("Failed to open local SQLite data"). Toolkit sessions now borrow the running server's database handle, which is what they always needed: they differ from the default session only in their plugin set. The unscoped `/mcp` endpoint was never affected.

.github/workflows/ci.yml

@@ -268,9 +268,11 @@ jobs:
           retention-days: 7
 
   e2e-local:
-    name: E2E (stdio MCP)
-    # Skipped on pull_request: the local scenario boots a real `executor web`
-    # plus a browser and is currently flaky on PRs. Still runs on push to main.
+    name: E2E (local MCP)
+    # Skipped on pull_request: these scenarios boot a real `executor web` and
+    # are currently flaky on PRs. Still runs on push to main — which is where
+    # the toolkit-MCP 500 and the `Bun is not defined` spawn failure would have
+    # been caught, had the step covered more than stdio-mcp.
     if: github.event_name != 'pull_request'
     runs-on: blacksmith-4vcpu-ubuntu-2404
     timeout-minutes: 20
@@ -315,14 +317,22 @@ jobs:
         working-directory: e2e
 
       # The `local` project is excluded from the default `test` chain (each
-      # scenario boots its own `executor web`). Run just the stdio MCP scenario
-      # here: it is the auto-connect / env-as-secret regression guard, and
-      # running it alone avoids the boot-resource accumulation and the
-      # pre-existing browser flakiness of the rest of the local suite. Expanding
-      # to the full `local` project (bun run test:local) is a follow-up once
-      # those are stabilized.
-      - name: Run the stdio MCP scenario
-        run: bunx vitest run --project local local/stdio-mcp.test.ts
+      # scenario boots its own `executor web`). Run the MCP-surface scenarios
+      # here rather than the whole project: these are the regression guards
+      # (stdio auto-connect / env-as-secret, toolkit-scoped sessions, the
+      # daemon-attach bridge, and native elicitation) and they need no browser,
+      # so they avoid the pre-existing browser flakiness of the rest of the
+      # local suite. Keeping them OUT of CI is how they rotted unnoticed:
+      # toolkit MCP 500'd from the day the data-dir lock landed, and the
+      # attach-stress file threw `Bun is not defined` on every run, because
+      # only stdio-mcp was ever exercised here.
+      - name: Run the local MCP scenarios
+        run: |
+          bunx vitest run --project local \
+            local/stdio-mcp.test.ts \
+            local/toolkits-mcp.test.ts \
+            local/mcp-native-elicitation.test.ts \
+            local/cli-mcp-daemon-attach-stress.test.ts
         working-directory: e2e
 
   desktop-smoke:

apps/local/src/executor.ts

@@ -19,7 +19,7 @@ import type { McpPluginExtension } from "@executor-js/plugin-mcp";
 import executorConfig from "../executor.config";
 import { localAnalytics } from "./analytics";
 import { localDataMigrations } from "./db/data-migrations";
-import { openOwnedLocalDatabase } from "./db/owned-database";
+import { openOwnedLocalDatabase, type OwnedLocalDatabase } from "./db/owned-database";
 
 interface ResolvedStorage {
   readonly dataDir: string;
@@ -56,6 +56,16 @@ type LocalPlugins = readonly AnyPlugin[];
 
 export interface LocalExecutorOptions {
   readonly activeToolkitSlug?: string;
+  /**
+   * Reuse an already-open owned database instead of opening (and locking) the
+   * data dir again. A toolkit-scoped MCP session differs from the default one
+   * only in its plugin set, so it must ride the running server's DB handle:
+   * `openOwnedLocalDatabase` takes an EXCLUSIVE lock, and a second open from
+   * inside the same process contends with the lock this process already holds.
+   * The borrowed handle is NOT closed when the derived executor disposes —
+   * whoever opened it still owns its lifetime.
+   */
+  readonly borrowedDb?: OwnedLocalDatabase;
 }
 
 const loadLocalPlugins = (options: LocalExecutorOptions = {}) =>
@@ -92,6 +102,10 @@ const loadLocalPlugins = (options: LocalExecutorOptions = {}) =>
 interface LocalExecutorBundle {
   readonly executor: Executor<LocalPlugins>;
   readonly plugins: LocalPlugins;
+  /** The owned DB this bundle opened (or borrowed). Surfaced so a
+   *  toolkit-scoped executor can ride the SAME handle instead of contending
+   *  with this process's own exclusive data-dir lock. */
+  readonly db: OwnedLocalDatabase;
   /** Where this daemon's web UI is reachable, resolved once at boot. Surfaced
    *  so callers building user-facing links (MCP artifact deep links) use the
    *  same origin the executor itself was configured with. */
@@ -151,23 +165,27 @@ const createLocalExecutorLayer = (options: LocalExecutorOptions = {}) => {
       const tenantId = makeTenantId(cwd);
       const tables = collectTables();
 
-      const owned = yield* Effect.acquireRelease(
-        Effect.tryPromise({
-          try: () =>
-            openOwnedLocalDatabase({
-              dataDir: storage.dataDir,
-              tables,
-              namespace: localNamespace,
-              tenantId,
+      // A borrowed handle is owned by its opener, so it is used as-is and left
+      // open on release; only a handle opened here is closed here.
+      const owned = options.borrowedDb
+        ? options.borrowedDb
+        : yield* Effect.acquireRelease(
+            Effect.tryPromise({
+              try: () =>
+                openOwnedLocalDatabase({
+                  dataDir: storage.dataDir,
+                  tables,
+                  namespace: localNamespace,
+                  tenantId,
+                }),
+              catch: (cause) =>
+                new LocalExecutorCreateError({
+                  message: CREATE_SQLITE_ERROR_MESSAGE,
+                  cause,
+                }),
             }),
-          catch: (cause) =>
-            new LocalExecutorCreateError({
-              message: CREATE_SQLITE_ERROR_MESSAGE,
-              cause,
-            }),
-        }),
-        (database) => Effect.promise(() => database.close()).pipe(Effect.ignore),
-      );
+            (database) => Effect.promise(() => database.close()).pipe(Effect.ignore),
+          );
       const sqlite = owned.db;
       const migration = owned.migration;
 
@@ -243,7 +261,7 @@ const createLocalExecutorLayer = (options: LocalExecutorOptions = {}) => {
           );
       }
 
-      return { executor, plugins, webBaseUrl };
+      return { executor, plugins, webBaseUrl, db: owned };
     }),
   );
 };
@@ -257,6 +275,7 @@ export const createExecutorHandle = async (options: LocalExecutorOptions = {}) =
     executor: bundle.executor,
     plugins: bundle.plugins,
     webBaseUrl: bundle.webBaseUrl,
+    db: bundle.db,
     dispose: async () => {
       await Effect.runPromise(Effect.ignore(bundle.executor.close()));
       await ignorePromiseFailure("disposeRuntime", () => runtime.dispose());

apps/local/src/main.ts

@@ -136,8 +136,13 @@ export const createServerHandlers = async (token: string): Promise<ServerHandler
             },
           };
         }
+        // Borrow the running server's DB handle: this process already holds the
+        // data dir's exclusive ownership lock, so opening it a second time here
+        // fails against ourselves. The toolkit executor differs only in its
+        // plugin set, and the borrowed handle stays open when it disposes.
         const handle = await createExecutorHandle({
           activeToolkitSlug: resource.slug,
+          borrowedDb: (await getExecutorBundle()).db,
         });
         const toolkitEngine = withExecutionAnalytics(
           createExecutionEngine({

e2e/local/auth.test.ts

@@ -36,8 +36,10 @@ scenario(
           await page.goto(url, { waitUntil: "domcontentloaded" });
           await page.getByRole("link", { name: "Secrets" }).first().waitFor({ timeout: 30_000 });
           // Integrations actually LOAD (the built-in Executor integration) — proves
-          // auth + data, not just the static shell.
-          await page.getByText("built-in").first().waitFor({ timeout: 30_000 });
+          // auth + data, not just the static shell. Matched on the row's stable
+          // testid: the list renders each integration's name + slug, never the
+          // literal "built-in" (that string is only an internal `kind`).
+          await page.getByTestId("integration-entry-executor").first().waitFor({ timeout: 30_000 });
           // The token is moved out of the URL and persisted to localStorage.
           expect(new URL(page.url()).searchParams.has("_token")).toBe(false);
           const stored = await page.evaluate(() => localStorage.getItem("executor.authToken"));
@@ -70,7 +72,7 @@ scenario(
           await page.getByRole("button", { name: "Connect" }).click();
           await page.getByRole("link", { name: "Secrets" }).first().waitFor({ timeout: 30_000 });
           // The reconnect fully restores — integrations LOAD, not a stale 401.
-          await page.getByText("built-in").first().waitFor({ timeout: 30_000 });
+          await page.getByTestId("integration-entry-executor").first().waitFor({ timeout: 30_000 });
         });
       }),
     );

e2e/local/cli-mcp-daemon-attach-stress.test.ts

@@ -21,11 +21,11 @@ import { expect } from "@effect/vitest";
 import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
 import { Effect } from "effect";
+import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
 import { mkdtempSync, readdirSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { fileURLToPath } from "node:url";
-import type { Subprocess } from "bun";
 
 import { scenario } from "../src/scenario";
 
@@ -34,7 +34,10 @@ const testScope = join(repoRoot, "apps/local");
 // Generous: a dev-mode daemon boots a Vite dev server, slow under machine load.
 const readyTimeoutMs = 150_000;
 
-type DaemonProc = Subprocess<"ignore", "pipe", "pipe">;
+// vitest runs this suite under NODE, not bun, so the daemon is spawned with
+// node:child_process (the rest of the e2e harness does the same). `Bun.spawn`
+// here threw `ReferenceError: Bun is not defined` on every run.
+type DaemonProc = ChildProcessWithoutNullStreams;
 
 const waitForDaemonReady = (
   proc: DaemonProc,
@@ -44,50 +47,38 @@ const waitForDaemonReady = (
     let stdoutBuffer = "";
     let stderrBuffer = "";
     let settled = false;
-    const decoder = new TextDecoder();
-    const stdout = proc.stdout.getReader();
-    const stderr = proc.stderr.getReader();
     const deadline = setTimeout(() => {
       if (settled) return;
       settled = true;
       // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- boundary: captured daemon stderr
       rejectReady(new Error(`daemon did not announce ready: ${stderrBuffer}`));
     }, readyTimeoutMs);
-    void (async () => {
-      while (true) {
-        const { value, done } = await stderr.read();
-        if (done) return;
-        stderrBuffer += decoder.decode(value);
-      }
-    })();
-    void (async () => {
-      while (true) {
-        const { value, done } = await stdout.read();
-        if (done) {
-          if (!settled) {
-            settled = true;
-            clearTimeout(deadline);
-            // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- boundary: captured daemon stderr
-            rejectReady(new Error(`daemon stdout closed before ready: ${stderrBuffer}`));
-          }
-          return;
-        }
-        stdoutBuffer += decoder.decode(value);
-        const match = /Daemon ready on http:\/\/(?:\[[^\]]+\]|[^:\s]+):(\d+)/.exec(stdoutBuffer);
-        if (match) {
-          settled = true;
-          clearTimeout(deadline);
-          resolveReady({ port: Number(match[1]), stderr: () => stderrBuffer });
-          return;
-        }
+    proc.stderr.on("data", (chunk: Buffer) => {
+      stderrBuffer += chunk.toString();
+    });
+    proc.stdout.on("data", (chunk: Buffer) => {
+      if (settled) return;
+      stdoutBuffer += chunk.toString();
+      const match = /Daemon ready on http:\/\/(?:\[[^\]]+\]|[^:\s]+):(\d+)/.exec(stdoutBuffer);
+      if (match) {
+        settled = true;
+        clearTimeout(deadline);
+        resolveReady({ port: Number(match[1]), stderr: () => stderrBuffer });
       }
-    })();
+    });
+    proc.stdout.on("close", () => {
+      if (settled) return;
+      settled = true;
+      clearTimeout(deadline);
+      // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- boundary: captured daemon stderr
+      rejectReady(new Error(`daemon stdout closed before ready: ${stderrBuffer}`));
+    });
   });
 
 const spawnDaemon = (dataDir: string): DaemonProc =>
-  Bun.spawn(
+  spawn(
+    "bun",
     [
-      "bun",
       "run",
       "dev:cli",
       "daemon",
@@ -103,17 +94,20 @@ const spawnDaemon = (dataDir: string): DaemonProc =>
     {
       cwd: repoRoot,
       env: { ...process.env, EXECUTOR_DATA_DIR: dataDir },
-      stdin: "ignore",
-      stdout: "pipe",
-      stderr: "pipe",
+      stdio: ["ignore", "pipe", "pipe"],
     },
-  );
+  ) as DaemonProc;
+
+const exited = (proc: DaemonProc): Promise<void> =>
+  proc.exitCode !== null || proc.signalCode !== null
+    ? Promise.resolve()
+    : new Promise((resolve) => proc.once("exit", () => resolve()));
 
 const stopProc = async (proc: DaemonProc): Promise<void> => {
-  if (proc.exitCode !== null) return;
+  if (proc.exitCode !== null || proc.signalCode !== null) return;
   proc.kill("SIGTERM");
-  await Promise.race([proc.exited, Bun.sleep(3000)]);
-  if (proc.exitCode === null) proc.kill("SIGKILL");
+  await Promise.race([exited(proc), new Promise<void>((resolve) => setTimeout(resolve, 3000))]);
+  if (proc.exitCode === null && proc.signalCode === null) proc.kill("SIGKILL");
 };
 
 const startForegroundDaemon = (dataDir: string) =>
@@ -322,7 +316,12 @@ scenario(
     );
 
     daemon.proc.kill("SIGKILL");
-    yield* Effect.promise(() => Promise.race([daemon.proc.exited, Bun.sleep(3000)]));
+    yield* Effect.promise(() =>
+      Promise.race([
+        exited(daemon.proc),
+        new Promise<void>((resolve) => setTimeout(resolve, 3000)),
+      ]),
+    );
 
     // The next call must settle (reject) quickly — a 10s bound well under the
     // scenario timeout catches a hang.
@@ -332,7 +331,7 @@ scenario(
           .callTool({ name: "execute", arguments: { code: "return 3" } })
           .then(() => "resolved" as const)
           .catch(() => "rejected" as const),
-        Bun.sleep(10_000).then(() => "timeout" as const),
+        new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 10_000)),
       ]),
     );
     // eslint-disable-next-line no-console