History

Add integration tests and update errors, comments

Griffin Evans committed 4 months ago

Commit c8cf1ce1 parent c26fb424 files · +411 −5Browse files

Unified · Split

4 files
  1. apps/cli/src/daemon-state.ts +14 −0
  2. apps/cli/src/local-server-manifest.ts +13 −0
  3. apps/cli/src/main.ts +5 −0
  4. apps/local/src/db/owned-database.test.ts +379 −5
apps/cli/src/daemon-state.ts+14−0
@@ -5,6 +5,20 @@ import { FileSystem, Path } from "effect"; · 4 unmodified lines
55 import type { PlatformError } from "effect/PlatformError";
66 import * as Effect from "effect/Effect";
77
8+// ---------------------------------------------------------------------------
9+// Daemon discovery + spawn-dedup state — hints, not DB safety.
10+//
11+// DaemonRecord (daemon-<host>-<port>.json) and DaemonPointer
12+// (daemon-active-<host>-<scope>.json) tell a CLI invocation which port/PID to
13+// attach to, and DaemonStartLock (daemon-active-<host>-<scope>.json.lock)
14+// avoids spawning daemons that would only lose the ownership race and die.
15+// None of them gate access to data.db: that is the data-dir ownership lock in
16+// @executor-js/local (apps/local/src/db/data-dir-ownership.ts), held for the
17+// life of the serving process. These files are pure optimization — if they are
18+// missing or stale, a redundant daemon may spawn, fail to acquire ownership,
19+// and exit; correctness is never at risk.
20+// ---------------------------------------------------------------------------
21+
822 // ---------------------------------------------------------------------------
923 // Types
1024 // ---------------------------------------------------------------------------
apps/cli/src/local-server-manifest.ts+13−0
@@ -10,6 +10,19 @@ import { · 9 unmodified lines
1010 type ExecutorLocalServerManifest,
1111 } from "@executor-js/sdk/shared";
1212
13+// ---------------------------------------------------------------------------
14+// server-control/server.json — a discovery/attach HINT, not an ownership proof.
15+//
16+// It records where a live local server is listening (origin + bearer) so other
17+// CLI invocations and the desktop app can attach instead of spawning a
18+// duplicate. The actual "only one process may open data.db" guarantee lives at
19+// the DB layer: the data-dir ownership lock in @executor-js/local
20+// (apps/local/src/db/data-dir-ownership.ts), acquired before any serving DB
21+// handle exists. If this manifest is missing, stale, or malformed, the worst
22+// outcome is a lost friendly attach — the kernel lock still refuses a second
23+// owner, so the database stays safe.
24+// ---------------------------------------------------------------------------
25+
1326 export const resolveExecutorDataDir = (path: Path.Path): string =>
1427 resolve(process.env.EXECUTOR_DATA_DIR ?? path.join(homedir(), ".executor"));
1528
apps/cli/src/main.ts+5−0
@@ -345,6 +345,11 @@ const makeLocalServerManifest = (input: { · 344 unmodified lines
345345 };
346346 });
347347
348+// Friendly, intentionally racy fast-path: it reads the server.json hint to fail
349+// early with a helpful message when another local server is already up. It is
350+// NOT the ownership gate — the DB ownership lock inside startServer
351+// (openOwnedLocalDatabase) is. A stale/missing manifest only costs the nice
352+// message; the kernel lock still refuses a second owner.
348353 const assertNoOtherActiveLocalServer = (): Effect.Effect<
349354 void,
350355 Error,
apps/local/src/db/owned-database.test.ts+379−5
@@ -1,5 +1,14 @@
1−/* oxlint-disable executor/no-try-catch-or-throw -- boundary: DB ownership test must close the held DB/lock handle even when assertions fail */
2−import { mkdtempSync, realpathSync, rmSync } from "node:fs";
1+/* oxlint-disable executor/no-try-catch-or-throw -- boundary: DB ownership tests must tear down held DB/lock handles and child processes even when assertions fail */
2+import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
3+import {
4+ existsSync,
5+ mkdirSync,
6+ mkdtempSync,
7+ realpathSync,
8+ rmSync,
9+ symlinkSync,
10+ writeFileSync,
11+} from "node:fs";
312 import { tmpdir } from "node:os";
413 import { join } from "node:path";
514
@@ -7,21 +16,33 @@ import { describe, expect, it } from "@effect/vitest"; · 1 unmodified line
716 import { collectTables } from "@executor-js/api/server";
817
918 import { acquireDataDirOwnership, DataDirOwnershipHeld } from "./data-dir-ownership";
19+import { executeSql, openLocalLibsql } from "./libsql";
1020 import { openOwnedLocalDatabase, type OwnedLocalDatabase } from "./owned-database";
1121
1222 const LOCK_DATABASE_FILENAME = "data.db.owner-lock";
23+const TEST_NAMESPACE = "executor_local_owned_database_test";
24+const appRoot = join(import.meta.dirname, "../..");
1325
14−const makeOwnedTestDatabase = (dataDir: string) =>
26+const delay = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
27+
28+const makeOwnedTestDatabase = (dataDir: string, namespace = TEST_NAMESPACE) =>
1529 openOwnedLocalDatabase({
1630 dataDir,
1731 tables: collectTables(),
18− namespace: "executor_local_owned_database_test",
32+ namespace,
1933 tenantId: "owned-database-test-tenant",
2034 });
2135
36+const makeWorkDir = (prefix: string): string =>
37+ mkdtempSync(join(tmpdir(), `executor-owned-database-${prefix}-`));
38+
39+// data.db exists only after the sole owner opens it, so it is the witness that a
40+// non-owner never wrote anything: an ownership loser must fail before any DB I/O.
41+const servingDbPath = (dataDir: string): string => join(realpathSync(dataDir), "data.db");
42+
2243 describe("openOwnedLocalDatabase", () => {
2344 it("holds data-dir ownership until the serving database is closed", async () => {
24− const workDir = mkdtempSync(join(tmpdir(), "executor-owned-database-"));
45+ const workDir = makeWorkDir("lifetime");
2546 const dataDir = join(workDir, "data");
2647 let owned: OwnedLocalDatabase | null = null;
2748
@@ -53,4 +74,357 @@ describe("openOwnedLocalDatabase", () => { · 25 unmodified lines
5374 rmSync(workDir, { recursive: true, force: true });
5475 }
5576 });
77+
78+ it("refuses to open (and never creates data.db) when ownership is already held", async () => {
79+ const workDir = makeWorkDir("gate-order");
80+ const dataDir = join(workDir, "data");
81+ const ownership = await acquireDataDirOwnership(dataDir);
82+
83+ try {
84+ await expect(makeOwnedTestDatabase(dataDir)).rejects.toBeInstanceOf(DataDirOwnershipHeld);
85+ // The gate runs before any migration or createSqliteFumaDb work, so the
86+ // loser cannot have written the serving database.
87+ expect(existsSync(servingDbPath(dataDir))).toBe(false);
88+ } finally {
89+ await ownership.release();
90+ }
91+
92+ // Once the owner releases, a fresh open succeeds and creates data.db.
93+ const owned = await makeOwnedTestDatabase(dataDir);
94+ try {
95+ expect(existsSync(owned.sqlitePath)).toBe(true);
96+ } finally {
97+ await owned.close();
98+ rmSync(workDir, { recursive: true, force: true });
99+ }
100+ });
101+
102+ it("keeps the gate independent of a deleted or corrupt server.json hint", async () => {
103+ const workDir = makeWorkDir("hint");
104+ const dataDir = join(workDir, "data");
105+ const owned = await makeOwnedTestDatabase(dataDir);
106+
107+ try {
108+ // server.json is only a coordination hint. A garbage manifest must not let
109+ // a second process open the DB while the real owner is alive.
110+ const serverControlDir = join(realpathSync(dataDir), "server-control");
111+ mkdirSync(serverControlDir, { recursive: true });
112+ const manifestPath = join(serverControlDir, "server.json");
113+ writeFileSync(manifestPath, "{ this is not valid json");
114+ await expect(makeOwnedTestDatabase(dataDir)).rejects.toBeInstanceOf(DataDirOwnershipHeld);
115+
116+ // Deleting the hint entirely changes nothing: the kernel lock is the gate.
117+ rmSync(manifestPath, { force: true });
118+ await expect(makeOwnedTestDatabase(dataDir)).rejects.toBeInstanceOf(DataDirOwnershipHeld);
119+ } finally {
120+ await owned.close();
121+ rmSync(workDir, { recursive: true, force: true });
122+ }
123+ });
124+
125+ it("collides on one ownership lock across symlinked data dirs", async () => {
126+ const workDir = makeWorkDir("symlink");
127+ const realDir = join(workDir, "real");
128+ const linkDir = join(workDir, "link");
129+ mkdirSync(realDir, { recursive: true });
130+ symlinkSync(realDir, linkDir, "dir");
131+
132+ const owned = await makeOwnedTestDatabase(realDir);
133+ try {
134+ const expectedLockPath = join(realpathSync(realDir), LOCK_DATABASE_FILENAME);
135+ expect(owned.lockPath).toBe(expectedLockPath);
136+
137+ // The symlinked path resolves to the same real inode, so it must lose the
138+ // ownership race rather than open a second handle on the same data.db.
139+ await expect(acquireDataDirOwnership(linkDir)).rejects.toMatchObject({
140+ _tag: "DataDirOwnershipHeld",
141+ lockPath: expectedLockPath,
142+ });
143+ } finally {
144+ await owned.close();
145+ rmSync(workDir, { recursive: true, force: true });
146+ }
147+ });
148+
149+ it("opens distinct real data dirs independently", async () => {
150+ const workDir = makeWorkDir("distinct");
151+ const dataDirA = join(workDir, "a");
152+ const dataDirB = join(workDir, "b");
153+
154+ const [ownedA, ownedB] = await Promise.all([
155+ makeOwnedTestDatabase(dataDirA),
156+ makeOwnedTestDatabase(dataDirB),
157+ ]);
158+ try {
159+ expect(ownedA.lockPath).not.toBe(ownedB.lockPath);
160+ expect(existsSync(ownedA.sqlitePath)).toBe(true);
161+ expect(existsSync(ownedB.sqlitePath)).toBe(true);
162+ } finally {
163+ await Promise.all([ownedA.close(), ownedB.close()]);
164+ rmSync(workDir, { recursive: true, force: true });
165+ }
166+ });
167+
168+ it("recovers a v1->v2 migration killed mid-flip under ownership", async () => {
169+ const workDir = makeWorkDir("migration");
170+ const dataDir = join(workDir, "data");
171+ mkdirSync(dataDir, { recursive: true });
172+ const realDataDir = realpathSync(dataDir);
173+ const sqlitePath = join(realDataDir, "data.db");
174+ const journalPath = `${sqlitePath}.v1-v2-migration.json`;
175+ const tenantId = "owned-migration-workspace";
176+ const marker = join(workDir, "paused-flip");
177+
178+ await seedMinimalV1Db(sqlitePath, tenantId);
179+
180+ // Pause mid-flip: at "canonical-moved" the original data.db has already been
181+ // moved aside to the backup and the staged v2 DB has not been swapped in
182+ // yet, so a SIGKILL here leaves a journal the next owner must RESUME (via
183+ // completeJournaledFlip) rather than re-run from scratch.
184+ const child = spawnOpenOwnedChild({
185+ dataDir,
186+ tenantId,
187+ pauseAt: "canonical-moved",
188+ pauseFile: marker,
189+ });
190+
191+ try {
192+ // Wait until the flip is mid-way under the child's ownership lock.
193+ const paused = await waitForMarker(marker, child, 30_000);
194+ expect(
195+ paused.markerFound,
196+ `child never reached the flip pause:\nstdout: ${child.stdout}\nstderr: ${child.stderr}`,
197+ ).toBe(true);
198+ // Mid-flip witness: the canonical data.db has been moved to the backup and
199+ // the staged copy is not yet installed.
200+ expect(existsSync(sqlitePath)).toBe(false);
201+
202+ // SIGKILL the holder so the OS releases the ownership lock mid-flip.
203+ child.child.kill("SIGKILL");
204+ await waitForChildClose(child, 5_000);
205+ expect(child.child.signalCode).toBe("SIGKILL");
206+
207+ // The journal proves the flip was interrupted; ownership must now be free
208+ // because the holder died.
209+ expect(existsSync(journalPath)).toBe(true);
210+ const reclaimed = await acquireDataDirOwnership(dataDir);
211+ await reclaimed.release();
212+
213+ // The next sole owner resumes the journaled flip during open (recovery
214+ // runs completeJournaledFlip before the serving connection opens). The
215+ // steady-state migrate then sees an already-v2 DB, so migration.migrated
216+ // is false even though the on-disk DB is now v2 — the cleared journal and
217+ // the v2 rows are the real proof.
218+ const owned = await openOwnedLocalDatabase({
219+ dataDir,
220+ tables: collectTables(),
221+ namespace: "executor_local",
222+ tenantId,
223+ });
224+ try {
225+ expect(existsSync(journalPath)).toBe(false);
226+ const integrations = await owned.db.client.execute("SELECT tenant, slug FROM integration");
227+ expect(integrations.rows).toEqual([{ tenant: tenantId, slug: "stripe_api" }]);
228+ } finally {
229+ await owned.close();
230+ }
231+ } finally {
232+ if (child.child.exitCode === null && child.child.signalCode === null) {
233+ child.child.kill("SIGKILL");
234+ await waitForChildClose(child, 5_000);
235+ }
236+ child.child.stdout.destroy();
237+ child.child.stderr.destroy();
238+ rmSync(workDir, { recursive: true, force: true });
239+ }
240+ });
56241 });
242+
243+// ---------------------------------------------------------------------------
244+// Minimal v1 fixture + child-process harness
245+// ---------------------------------------------------------------------------
246+
247+// A purpose-built v1-final database with exactly the tables readV1Snapshot
248+// reads. plugin_storage is present so the legacy schema replay is skipped, and
249+// a single openapi source makes isLocalV1Database return true and gives the
250+// migration real content to carry into v2.
251+const seedMinimalV1Db = async (dbPath: string, scopeId: string): Promise<void> => {
252+ const client = await openLocalLibsql(dbPath);
253+ // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: close the SQLite client even if seeding fails
254+ try {
255+ await client.execute("PRAGMA foreign_keys = OFF");
256+ await client.execute(
257+ "CREATE TABLE source (id text NOT NULL, scope_id text NOT NULL, plugin_id text NOT NULL, kind text NOT NULL, name text NOT NULL, PRIMARY KEY(scope_id, id))",
258+ );
259+ await client.execute(
260+ "CREATE TABLE plugin_storage (id text NOT NULL, scope_id text NOT NULL, plugin_id text NOT NULL, collection text NOT NULL, key text NOT NULL, data text NOT NULL, created_at integer NOT NULL, updated_at integer NOT NULL, PRIMARY KEY(scope_id, id))",
261+ );
262+ await client.execute(
263+ "CREATE TABLE credential_binding (id text NOT NULL, scope_id text NOT NULL, plugin_id text NOT NULL, source_id text NOT NULL, source_scope_id text NOT NULL, slot_key text NOT NULL, kind text NOT NULL, text_value text, secret_id text, connection_id text, created_at integer NOT NULL, updated_at integer NOT NULL, PRIMARY KEY(scope_id, id))",
264+ );
265+ await client.execute(
266+ "CREATE TABLE secret (id text NOT NULL, scope_id text NOT NULL, name text NOT NULL, provider text NOT NULL, owned_by_connection_id text, created_at integer NOT NULL, PRIMARY KEY(scope_id, id))",
267+ );
268+ await client.execute(
269+ "CREATE TABLE connection (id text NOT NULL, scope_id text NOT NULL, provider text NOT NULL, identity_label text, access_token_secret_id text, refresh_token_secret_id text, expires_at integer, provider_state text, PRIMARY KEY(scope_id, id))",
270+ );
271+ await client.execute(
272+ "CREATE TABLE tool_policy (id text NOT NULL, scope_id text NOT NULL, pattern text NOT NULL, action text NOT NULL, position text NOT NULL, created_at integer NOT NULL, updated_at integer NOT NULL, PRIMARY KEY(scope_id, id))",
273+ );
274+ await client.execute(
275+ "CREATE TABLE tool (id text NOT NULL, scope_id text NOT NULL, source_id text NOT NULL, plugin_id text NOT NULL, name text NOT NULL, description text NOT NULL, input_schema text, output_schema text, created_at integer NOT NULL, updated_at integer NOT NULL, PRIMARY KEY(scope_id, id))",
276+ );
277+
278+ const now = Date.now();
279+ await executeSql(
280+ client,
281+ "INSERT INTO source (id, scope_id, plugin_id, kind, name) VALUES (?, ?, ?, ?, ?)",
282+ ["stripe_api", scopeId, "openapi", "openapi", "Stripe"],
283+ );
284+ await executeSql(
285+ client,
286+ "INSERT INTO plugin_storage (id, scope_id, plugin_id, collection, key, data, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
287+ [
288+ "openapi-source-stripe",
289+ scopeId,
290+ "openapi",
291+ "source",
292+ "stripe_api",
293+ JSON.stringify({
294+ config: {
295+ spec: "{}",
296+ headers: {
297+ Authorization: { kind: "binding", slot: "header:authorization", prefix: "Bearer " },
298+ },
299+ },
300+ }),
301+ now,
302+ now,
303+ ],
304+ );
305+ await executeSql(
306+ client,
307+ "INSERT INTO credential_binding (id, scope_id, plugin_id, source_id, source_scope_id, slot_key, kind, text_value, secret_id, connection_id, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
308+ [
309+ "stripe-auth",
310+ scopeId,
311+ "openapi",
312+ "stripe_api",
313+ scopeId,
314+ "header:authorization",
315+ "secret",
316+ null,
317+ "stripe-key",
318+ null,
319+ now,
320+ now,
321+ ],
322+ );
323+ await executeSql(
324+ client,
325+ "INSERT INTO secret (id, scope_id, name, provider, owned_by_connection_id, created_at) VALUES (?, ?, ?, ?, ?, ?)",
326+ ["stripe-key", scopeId, "Stripe key", "file", null, now],
327+ );
328+ } finally {
329+ client.close();
330+ }
331+};
332+
333+interface OwnedChild {
334+ readonly child: ChildProcessWithoutNullStreams;
335+ stdout: string;
336+ stderr: string;
337+}
338+
339+interface ChildCloseResult {
340+ readonly markerFound: boolean;
341+ readonly exitCode: number | null;
342+ readonly signalCode: NodeJS.Signals | null;
343+}
344+
345+const spawnOpenOwnedChild = (input: {
346+ readonly dataDir: string;
347+ readonly tenantId: string;
348+ readonly pauseAt: string;
349+ readonly pauseFile: string;
350+}): OwnedChild => {
351+ const code = `
352+ import { collectTables } from "@executor-js/api/server";
353+ import { openOwnedLocalDatabase } from "./src/db/owned-database.ts";
354+ const owned = await openOwnedLocalDatabase({
355+ dataDir: process.env.EXECUTOR_TEST_DATA_DIR,
356+ tables: collectTables(),
357+ namespace: "executor_local",
358+ tenantId: process.env.EXECUTOR_TEST_TENANT,
359+ });
360+ console.log("OPENED");
361+ await owned.close();
362+ `;
363+ const child = spawn(process.execPath, ["-e", code], {
364+ cwd: appRoot,
365+ env: {
366+ ...process.env,
367+ NODE_ENV: "test",
368+ EXECUTOR_TEST_DATA_DIR: input.dataDir,
369+ EXECUTOR_TEST_TENANT: input.tenantId,
370+ EXECUTOR_V1_V2_MIGRATION_PAUSE_AT: input.pauseAt,
371+ EXECUTOR_V1_V2_MIGRATION_PAUSE_FILE: input.pauseFile,
372+ },
373+ stdio: "pipe",
374+ });
375+ child.unref();
376+
377+ const owned: OwnedChild = { child, stdout: "", stderr: "" };
378+ child.stdout.setEncoding("utf-8");
379+ child.stderr.setEncoding("utf-8");
380+ child.stdout.on("data", (chunk: string) => {
381+ owned.stdout += chunk;
382+ });
383+ child.stderr.on("data", (chunk: string) => {
384+ owned.stderr += chunk;
385+ });
386+ return owned;
387+};
388+
389+const waitForMarker = async (
390+ markerPath: string,
391+ owned: OwnedChild,
392+ timeoutMs: number,
393+): Promise<ChildCloseResult> => {
394+ const deadline = Date.now() + timeoutMs;
395+ while (Date.now() < deadline) {
396+ if (existsSync(markerPath)) {
397+ return {
398+ markerFound: true,
399+ exitCode: owned.child.exitCode,
400+ signalCode: owned.child.signalCode,
401+ };
402+ }
403+ if (owned.child.exitCode !== null || owned.child.signalCode !== null) {
404+ return {
405+ markerFound: false,
406+ exitCode: owned.child.exitCode,
407+ signalCode: owned.child.signalCode,
408+ };
409+ }
410+ await delay(25);
411+ }
412+ return { markerFound: false, exitCode: owned.child.exitCode, signalCode: owned.child.signalCode };
413+};
414+
415+const waitForChildClose = async (owned: OwnedChild, timeoutMs: number): Promise<void> =>
416+ new Promise((resolve) => {
417+ if (owned.child.exitCode !== null || owned.child.signalCode !== null) {
418+ resolve();
419+ return;
420+ }
421+ const timer = setTimeout(() => {
422+ owned.child.off("close", onClose);
423+ resolve();
424+ }, timeoutMs);
425+ const onClose = () => {
426+ clearTimeout(timer);
427+ resolve();
428+ };
429+ owned.child.once("close", onClose);
430+ });