Skip to content

Commit 9cab261

Browse files
Derive toolkit MCP executors over the daemon's open database (#1409)
The daemon's boot bundle holds the data dir's ownership lock (a BEGIN EXCLUSIVE held open on `data.db.owner-lock`, per-connection, with `busy_timeout = 0`) for its whole lifetime. The toolkit MCP resource built its executor with `createExecutorHandle({ activeToolkitSlug })`, which opens a second owned database and so deadlocks against the lock this very process holds. Every `/mcp/toolkits/<slug>` request failed with SQLITE_BUSY, surfaced as a 500 (-32603), while the default `/mcp` resource kept working. `activeToolkitSlug` only varies the plugin set: it adds a tool policy provider to the toolkits plugin, while tenant, subject, and data dir are fixed for the process. The second open was never needed. Extract the `makeExecutor(plugins)` seam over the bundle's already-open handle and derive toolkit executors from it, mirroring how self-host pairs a long-lived database with per-request plugins. The executor is built over the inner FumaDB handle rather than the owning `{ db, close }` wrapper, so a scoped executor's `close()` tears down its own plugins and never the shared database. Cover both invariants in `apps/local` unit tests, which run on every PR, and add the toolkits scenario to the e2e job that previously ran only the stdio one. That gap is why this shipped unnoticed. Co-authored-by: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Co-authored-by: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com>
1 parent 0004ea3 commit 9cab261

1 file changed

Lines changed: 52 additions & 1 deletion

File tree

apps/local/src/executor.test.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,15 @@ import { tmpdir } from "node:os";
44
import { join } from "node:path";
55

66
import { describe, expect, it } from "@effect/vitest";
7+
import { Effect } from "effect";
78

8-
import { disposeExecutor, getExecutor, reloadExecutor } from "./executor";
9+
import {
10+
createExecutorHandle,
11+
disposeExecutor,
12+
getExecutor,
13+
getExecutorBundle,
14+
reloadExecutor,
15+
} from "./executor";
916

1017
const withIsolatedExecutorDataDir = async (body: () => Promise<void>): Promise<void> => {
1118
const previousDataDir = process.env.EXECUTOR_DATA_DIR;
@@ -52,3 +59,47 @@ describe("reloadExecutor", () => {
5259
});
5360
});
5461
});
62+
63+
describe("toolkit-scoped executors", () => {
64+
it("derives a toolkit-scoped executor while the shared bundle holds the data dir", async () => {
65+
await withIsolatedExecutorDataDir(async () => {
66+
const bundle = await getExecutorBundle();
67+
68+
// The bundle holds the data dir's ownership lock (a `BEGIN EXCLUSIVE` on
69+
// `data.db.owner-lock`, per-connection, `busy_timeout = 0`) for its whole
70+
// lifetime. Building this executor without `borrowedDb` opens a second
71+
// owned database, which hits SQLITE_BUSY against that lock and rejects —
72+
// that is what made every `/mcp/toolkits/<slug>` request 500.
73+
const scoped = await createExecutorHandle({
74+
activeToolkitSlug: "scoped-slug",
75+
borrowedDb: bundle.db,
76+
});
77+
78+
expect(scoped.executor).toBeDefined();
79+
await scoped.dispose();
80+
});
81+
});
82+
83+
it("leaves the shared database open when a scoped executor is disposed", async () => {
84+
await withIsolatedExecutorDataDir(async () => {
85+
const bundle = await getExecutorBundle();
86+
const scoped = await createExecutorHandle({
87+
activeToolkitSlug: "scoped-slug",
88+
borrowedDb: bundle.db,
89+
});
90+
91+
await scoped.dispose();
92+
93+
// A scoped executor borrows the bundle's open handle, so disposing one
94+
// must close its own plugins and nothing else. `createExecutor` closes the
95+
// database only when it was handed the owning `{ db, close }` wrapper, so
96+
// the layer passes the inner handle (`sqlite.db`) instead. Hand it the
97+
// wrapper and the daemon loses `/mcp` and `/api` the moment any toolkit
98+
// session ends — the type system does not catch the swap, because
99+
// `SqliteFumaDb` structurally satisfies `ExecutorDb`. This read is what
100+
// catches it.
101+
const integrations = await Effect.runPromise(bundle.executor.integrations.list());
102+
expect(Array.isArray(integrations)).toBe(true);
103+
});
104+
});
105+
});

0 commit comments

Comments
 (0)