Merge pull request #1829 from paperclipai/pr/pap-891-worktree-reliability

fix(worktree): harden provisioned worktree isolation and test fallback behavior

packages/db/src/client.test.ts

@@ -1,83 +1,24 @@
 import { createHash } from "node:crypto";
 import fs from "node:fs";
-import net from "node:net";
-import os from "node:os";
-import path from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
 import postgres from "postgres";
 import {
   applyPendingMigrations,
-  ensurePostgresDatabase,
   inspectMigrations,
 } from "./client.js";
+import {
+  getEmbeddedPostgresTestSupport,
+  startEmbeddedPostgresTestDatabase,
+} from "./test-embedded-postgres.js";
 
-type EmbeddedPostgresInstance = {
-  initialise(): Promise<void>;
-  start(): Promise<void>;
-  stop(): Promise<void>;
-};
-
-type EmbeddedPostgresCtor = new (opts: {
-  databaseDir: string;
-  user: string;
-  password: string;
-  port: number;
-  persistent: boolean;
-  initdbFlags?: string[];
-  onLog?: (message: unknown) => void;
-  onError?: (message: unknown) => void;
-}) => EmbeddedPostgresInstance;
-
-const tempPaths: string[] = [];
-const runningInstances: EmbeddedPostgresInstance[] = [];
-
-async function getEmbeddedPostgresCtor(): Promise<EmbeddedPostgresCtor> {
-  const mod = await import("embedded-postgres");
-  return mod.default as EmbeddedPostgresCtor;
-}
-
-async function getAvailablePort(): Promise<number> {
-  return await new Promise((resolve, reject) => {
-    const server = net.createServer();
-    server.unref();
-    server.on("error", reject);
-    server.listen(0, "127.0.0.1", () => {
-      const address = server.address();
-      if (!address || typeof address === "string") {
-        server.close(() => reject(new Error("Failed to allocate test port")));
-        return;
-      }
-      const { port } = address;
-      server.close((error) => {
-        if (error) reject(error);
-        else resolve(port);
-      });
-    });
-  });
-}
+const cleanups: Array<() => Promise<void>> = [];
+const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
+const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
 
 async function createTempDatabase(): Promise<string> {
-  const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-db-client-"));
-  tempPaths.push(dataDir);
-  const port = await getAvailablePort();
-  const EmbeddedPostgres = await getEmbeddedPostgresCtor();
-  const instance = new EmbeddedPostgres({
-    databaseDir: dataDir,
-    user: "paperclip",
-    password: "paperclip",
-    port,
-    persistent: true,
-    initdbFlags: ["--encoding=UTF8", "--locale=C", "--lc-messages=C"],
-    onLog: () => {},
-    onError: () => {},
-  });
-  await instance.initialise();
-  await instance.start();
-  runningInstances.push(instance);
-
-  const adminUrl = `postgres://paperclip:paperclip@127.0.0.1:${port}/postgres`;
-  await ensurePostgresDatabase(adminUrl, "paperclip");
-  return `postgres://paperclip:paperclip@127.0.0.1:${port}/paperclip`;
+  const db = await startEmbeddedPostgresTestDatabase("paperclip-db-client-");
+  cleanups.push(db.cleanup);
+  return db.connectionString;
 }
 
 async function migrationHash(migrationFile: string): Promise<string> {
@@ -89,19 +30,19 @@ async function migrationHash(migrationFile: string): Promise<string> {
 }
 
 afterEach(async () => {
-  while (runningInstances.length > 0) {
-    const instance = runningInstances.pop();
-    if (!instance) continue;
-    await instance.stop();
-  }
-  while (tempPaths.length > 0) {
-    const tempPath = tempPaths.pop();
-    if (!tempPath) continue;
-    fs.rmSync(tempPath, { recursive: true, force: true });
+  while (cleanups.length > 0) {
+    const cleanup = cleanups.pop();
+    await cleanup?.();
   }
 });
 
-describe("applyPendingMigrations", () => {
+if (!embeddedPostgresSupport.supported) {
+  console.warn(
+    `Skipping embedded Postgres migration tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
+  );
+}
+
+describeEmbeddedPostgres("applyPendingMigrations", () => {
   it(
     "applies an inserted earlier migration without replaying later legacy migrations",
     async () => {

packages/db/src/index.ts

@@ -11,6 +11,12 @@ export {
   type MigrationBootstrapResult,
   type Db,
 } from "./client.js";
+export {
+  getEmbeddedPostgresTestSupport,
+  startEmbeddedPostgresTestDatabase,
+  type EmbeddedPostgresTestDatabase,
+  type EmbeddedPostgresTestSupport,
+} from "./test-embedded-postgres.js";
 export {
   runDatabaseBackup,
   runDatabaseRestore,

packages/db/src/test-embedded-postgres.ts

@@ -0,0 +1,144 @@
+import fs from "node:fs";
+import net from "node:net";
+import os from "node:os";
+import path from "node:path";
+import { applyPendingMigrations, ensurePostgresDatabase } from "./client.js";
+
+type EmbeddedPostgresInstance = {
+  initialise(): Promise<void>;
+  start(): Promise<void>;
+  stop(): Promise<void>;
+};
+
+type EmbeddedPostgresCtor = new (opts: {
+  databaseDir: string;
+  user: string;
+  password: string;
+  port: number;
+  persistent: boolean;
+  initdbFlags?: string[];
+  onLog?: (message: unknown) => void;
+  onError?: (message: unknown) => void;
+}) => EmbeddedPostgresInstance;
+
+export type EmbeddedPostgresTestSupport = {
+  supported: boolean;
+  reason?: string;
+};
+
+export type EmbeddedPostgresTestDatabase = {
+  connectionString: string;
+  cleanup(): Promise<void>;
+};
+
+let embeddedPostgresSupportPromise: Promise<EmbeddedPostgresTestSupport> | null = null;
+
+async function getEmbeddedPostgresCtor(): Promise<EmbeddedPostgresCtor> {
+  const mod = await import("embedded-postgres");
+  return mod.default as EmbeddedPostgresCtor;
+}
+
+async function getAvailablePort(): Promise<number> {
+  return await new Promise((resolve, reject) => {
+    const server = net.createServer();
+    server.unref();
+    server.on("error", reject);
+    server.listen(0, "127.0.0.1", () => {
+      const address = server.address();
+      if (!address || typeof address === "string") {
+        server.close(() => reject(new Error("Failed to allocate test port")));
+        return;
+      }
+      const { port } = address;
+      server.close((error) => {
+        if (error) reject(error);
+        else resolve(port);
+      });
+    });
+  });
+}
+
+function formatEmbeddedPostgresError(error: unknown): string {
+  if (error instanceof Error && error.message.length > 0) return error.message;
+  if (typeof error === "string" && error.length > 0) return error;
+  return "embedded Postgres startup failed";
+}
+
+async function probeEmbeddedPostgresSupport(): Promise<EmbeddedPostgresTestSupport> {
+  const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-embedded-postgres-probe-"));
+  const port = await getAvailablePort();
+  const EmbeddedPostgres = await getEmbeddedPostgresCtor();
+  const instance = new EmbeddedPostgres({
+    databaseDir: dataDir,
+    user: "paperclip",
+    password: "paperclip",
+    port,
+    persistent: true,
+    initdbFlags: ["--encoding=UTF8", "--locale=C", "--lc-messages=C"],
+    onLog: () => {},
+    onError: () => {},
+  });
+
+  try {
+    await instance.initialise();
+    await instance.start();
+    return { supported: true };
+  } catch (error) {
+    return {
+      supported: false,
+      reason: formatEmbeddedPostgresError(error),
+    };
+  } finally {
+    await instance.stop().catch(() => {});
+    fs.rmSync(dataDir, { recursive: true, force: true });
+  }
+}
+
+export async function getEmbeddedPostgresTestSupport(): Promise<EmbeddedPostgresTestSupport> {
+  if (!embeddedPostgresSupportPromise) {
+    embeddedPostgresSupportPromise = probeEmbeddedPostgresSupport();
+  }
+  return await embeddedPostgresSupportPromise;
+}
+
+export async function startEmbeddedPostgresTestDatabase(
+  tempDirPrefix: string,
+): Promise<EmbeddedPostgresTestDatabase> {
+  const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), tempDirPrefix));
+  const port = await getAvailablePort();
+  const EmbeddedPostgres = await getEmbeddedPostgresCtor();
+  const instance = new EmbeddedPostgres({
+    databaseDir: dataDir,
+    user: "paperclip",
+    password: "paperclip",
+    port,
+    persistent: true,
+    initdbFlags: ["--encoding=UTF8", "--locale=C", "--lc-messages=C"],
+    onLog: () => {},
+    onError: () => {},
+  });
+
+  try {
+    await instance.initialise();
+    await instance.start();
+
+    const adminConnectionString = `postgres://paperclip:paperclip@127.0.0.1:${port}/postgres`;
+    await ensurePostgresDatabase(adminConnectionString, "paperclip");
+    const connectionString = `postgres://paperclip:paperclip@127.0.0.1:${port}/paperclip`;
+    await applyPendingMigrations(connectionString);
+
+    return {
+      connectionString,
+      cleanup: async () => {
+        await instance.stop().catch(() => {});
+        fs.rmSync(dataDir, { recursive: true, force: true });
+      },
+    };
+  } catch (error) {
+    await instance.stop().catch(() => {});
+    fs.rmSync(dataDir, { recursive: true, force: true });
+    throw new Error(
+      `Failed to start embedded PostgreSQL test database: ${formatEmbeddedPostgresError(error)}`,
+    );
+  }
+}