DB File Lock (rebase of #1050 by @grfwings) (#1175)

* Kill daemon child if parent times out

* Safer migration

* Make flip idempotent, remove reduntant db copies, fix auth.json mutating eraly, derive expected counts from plan arrays

* Fix various correctness issues

* Add sqlite lock db ownership primitive and test

* Use pathToFile not string concatenation for dir

* Add owned-database.ts entry point

* Implement new database ownership process in cli and desktop sidecar

* Delete startup lock and v1->v2 migration lock

* Remove double-probe from v1-v2 migration

* Add integration tests and update errors, comments

* Don't change scope -> tenant mappings

* Add polling to desktop sidecar and fix infinite loop in findDataDirOwnershipHeld

* Inculde keychainBackups in migration journal

* Add guard to JSON.parse in readMigrationJournal

* Add coverage for corrupt journals, increase timeout for SIGKILL recovery sweep

* readMigrationJournal now distinguishes missing, valid, and unreadable journals

* writeSidecarManifest should create server-control before writing server.json

* Desktop can't attach to CLI daemon because it might be foreground; revert to old behavior

* Add test for SIGKILL after journal write and after secret write

* Shared executor should wait behind a lifecycle hook so getExecutor() cannot reopen while a previous handle is still disposing

* Fix loadSharedHandle being able to poison future getExecutor() calls

---------

Co-authored-by: Griffin Evans <griffin@ibm.com>
Co-authored-by: Griffin <38366015+grfwings@users.noreply.github.com>

Author: 3 people

apps/cli/src/daemon-state.ts

@@ -5,6 +5,20 @@ import { FileSystem, Path } from "effect";
 import type { PlatformError } from "effect/PlatformError";
 import * as Effect from "effect/Effect";
 
+// ---------------------------------------------------------------------------
+// Daemon discovery + spawn-dedup state — hints, not DB safety.
+//
+// DaemonRecord (daemon-<host>-<port>.json) and DaemonPointer
+// (daemon-active-<host>-<scope>.json) tell a CLI invocation which port/PID to
+// attach to, and DaemonStartLock (daemon-active-<host>-<scope>.json.lock)
+// avoids spawning daemons that would only lose the ownership race and die.
+// None of them gate access to data.db: that is the data-dir ownership lock in
+// @executor-js/local (apps/local/src/db/data-dir-ownership.ts), held for the
+// life of the serving process. These files are pure optimization — if they are
+// missing or stale, a redundant daemon may spawn, fail to acquire ownership,
+// and exit; correctness is never at risk.
+// ---------------------------------------------------------------------------
+
 // ---------------------------------------------------------------------------
 // Types
 // ---------------------------------------------------------------------------

apps/cli/src/daemon.test.ts

@@ -1,13 +1,18 @@
 import { describe, expect, it } from "@effect/vitest";
+import { existsSync, mkdtempSync, rmSync } from "node:fs";
 import { createServer, type Server } from "node:http";
 import type { AddressInfo } from "node:net";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import * as Effect from "effect/Effect";
 
 import {
   canAutoStartLocalDaemonForHost,
   isDevCliEntrypoint,
   isExecutorServerReachable,
   planServiceInstall,
+  spawnDetached,
+  terminateSpawnedDetachedProcess,
 } from "./daemon";
 
 describe("isDevCliEntrypoint", () => {
@@ -50,6 +55,48 @@ describe("canAutoStartLocalDaemonForHost", () => {
   });
 });
 
+const waitForFile = (path: string): Effect.Effect<boolean> =>
+  Effect.gen(function* () {
+    for (let attempt = 0; attempt < 40; attempt++) {
+      if (existsSync(path)) return true;
+      yield* Effect.promise(() => new Promise((resolve) => setTimeout(resolve, 50)));
+    }
+    return false;
+  });
+
+describe("spawnDetached", () => {
+  it.effect("can terminate the spawned detached process", () =>
+    Effect.gen(function* () {
+      const workDir = mkdtempSync(join(tmpdir(), "executor-daemon-spawn-"));
+      const readyMarker = join(workDir, "ready");
+      const terminatedMarker = join(workDir, "terminated");
+
+      try {
+        const child = yield* Effect.acquireRelease(
+          spawnDetached({
+            command: process.execPath,
+            args: [
+              "-e",
+              "const fs = require('node:fs'); fs.writeFileSync(process.env.READY_FILE, 'ok'); process.on('SIGTERM', () => { fs.writeFileSync(process.env.TERMINATED_FILE, 'ok'); process.exit(0); }); setInterval(() => {}, 1000)",
+            ],
+            env: { ...process.env, READY_FILE: readyMarker, TERMINATED_FILE: terminatedMarker },
+          }),
+          (child) => terminateSpawnedDetachedProcess(child).pipe(Effect.ignore),
+        );
+
+        expect(child.pid).toBeGreaterThan(0);
+        const ready = yield* waitForFile(readyMarker);
+        expect(ready).toBe(true);
+        yield* terminateSpawnedDetachedProcess(child);
+        const terminated = yield* waitForFile(terminatedMarker);
+        expect(terminated).toBe(true);
+      } finally {
+        rmSync(workDir, { recursive: true, force: true });
+      }
+    }),
+  );
+});
+
 describe("isExecutorServerReachable", () => {
   it.effect("probes the unauthenticated /api/health endpoint without forwarding a credential", () =>
     Effect.gen(function* () {

apps/cli/src/daemon.ts

@@ -17,6 +17,10 @@ export interface DaemonSpawnSpec {
   readonly args: ReadonlyArray<string>;
 }
 
+export interface SpawnedDetachedProcess {
+  readonly pid: number;
+}
+
 export interface ExecutorServerReachabilityInput {
   readonly baseUrl: string;
 }
@@ -140,7 +144,7 @@ export const spawnDetached = (input: {
   readonly command: string;
   readonly args: ReadonlyArray<string>;
   readonly env: Record<string, string | undefined>;
-}): Effect.Effect<void, Error> =>
+}): Effect.Effect<SpawnedDetachedProcess, Error> =>
   Effect.try({
     try: () => {
       const child = spawn(input.command, [...input.args], {
@@ -149,13 +153,36 @@ export const spawnDetached = (input: {
         env: input.env,
       });
       child.unref();
+      if (typeof child.pid !== "number") {
+        child.kill();
+        throw new Error("Failed to spawn daemon process: child pid was not assigned");
+      }
+      return { pid: child.pid };
     },
     catch: (cause) =>
       cause instanceof Error
         ? cause
         : new Error(`Failed to spawn daemon process: ${String(cause)}`),
   });
 
+const signalPid = (pid: number): Effect.Effect<void, Error> =>
+  Effect.try({
+    try: () => {
+      process.kill(pid, "SIGTERM");
+    },
+    catch: (cause) =>
+      cause instanceof Error
+        ? cause
+        : new Error(`Failed to terminate daemon process ${pid}: ${String(cause)}`),
+  });
+
+export const terminateSpawnedDetachedProcess = (
+  child: SpawnedDetachedProcess,
+): Effect.Effect<void, Error> =>
+  process.platform === "win32"
+    ? signalPid(child.pid)
+    : signalPid(-child.pid).pipe(Effect.catch(() => signalPid(child.pid)));
+
 const waitForCondition = <E, R>(input: {
   readonly check: Effect.Effect<boolean, E, R>;
   readonly expected: boolean;

apps/cli/src/local-server-manifest.test.ts

@@ -5,13 +5,10 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { Path } from "effect";
 import * as Effect from "effect/Effect";
-import * as Exit from "effect/Exit";
 
 import { normalizeExecutorServerConnection } from "@executor-js/sdk/shared";
 import {
-  acquireLocalServerStartLock,
   readLocalServerManifest,
-  releaseLocalServerStartLock,
   removeLocalServerManifestIfOwnedBy,
   resolveExecutorDataDir,
   writeLocalServerManifest,
@@ -65,25 +62,6 @@ describe("local server manifest", () => {
     }).pipe(Effect.provide(BunServices.layer)),
   );
 
-  it.effect("serializes local startup with a stale-aware lock", () =>
-    Effect.gen(function* () {
-      const dataDir = mkdtempSync(join(tmpdir(), "executor-local-server-lock-"));
-      process.env.EXECUTOR_DATA_DIR = dataDir;
-
-      try {
-        const first = yield* acquireLocalServerStartLock();
-        const second = yield* Effect.exit(acquireLocalServerStartLock());
-
-        expect(Exit.isFailure(second)).toBe(true);
-        yield* releaseLocalServerStartLock(first);
-        const third = yield* acquireLocalServerStartLock();
-        yield* releaseLocalServerStartLock(third);
-      } finally {
-        rmSync(dataDir, { recursive: true, force: true });
-      }
-    }).pipe(Effect.provide(BunServices.layer)),
-  );
-
   it.effect("resolves the data dir from EXECUTOR_DATA_DIR", () =>
     Effect.gen(function* () {
       const dataDir = mkdtempSync(join(tmpdir(), "executor-local-server-dir-"));

apps/cli/src/local-server-manifest.ts

@@ -1,6 +1,6 @@
 import { homedir } from "node:os";
 import { resolve } from "node:path";
-import { FileSystem, Option, Path, Schema } from "effect";
+import { FileSystem, Path } from "effect";
 import type { PlatformError } from "effect/PlatformError";
 import * as Effect from "effect/Effect";
 
@@ -9,11 +9,19 @@ import {
   serializeExecutorLocalServerManifest,
   type ExecutorLocalServerManifest,
 } from "@executor-js/sdk/shared";
-import { isPidAlive } from "./daemon-state";
 
-export interface LocalServerStartLock {
-  readonly path: string;
-}
+// ---------------------------------------------------------------------------
+// server-control/server.json — a discovery/attach HINT, not an ownership proof.
+//
+// It records where a live local server is listening (origin + bearer) so other
+// CLI invocations and the desktop app can attach instead of spawning a
+// duplicate. The actual "only one process may open data.db" guarantee lives at
+// the DB layer: the data-dir ownership lock in @executor-js/local
+// (apps/local/src/db/data-dir-ownership.ts), acquired before any serving DB
+// handle exists. If this manifest is missing, stale, or malformed, the worst
+// outcome is a lost friendly attach — the kernel lock still refuses a second
+// owner, so the database stays safe.
+// ---------------------------------------------------------------------------
 
 export const resolveExecutorDataDir = (path: Path.Path): string =>
   resolve(process.env.EXECUTOR_DATA_DIR ?? path.join(homedir(), ".executor"));
@@ -24,9 +32,6 @@ const serverControlDir = (path: Path.Path): string =>
 const localServerManifestPath = (path: Path.Path): string =>
   path.join(serverControlDir(path), "server.json");
 
-const localServerStartLockPath = (path: Path.Path): string =>
-  path.join(serverControlDir(path), "startup.lock");
-
 export const readLocalServerManifest = (): Effect.Effect<
   ExecutorLocalServerManifest | null,
   never,
@@ -94,61 +99,3 @@ export const removeLocalServerManifest = (): Effect.Effect<
     const path = yield* Path.Path;
     yield* fs.remove(localServerManifestPath(path), { force: true });
   });
-
-const StartupLockPayload = Schema.Struct({
-  pid: Schema.Number,
-});
-
-const decodeStartupLockPayload = Schema.decodeUnknownOption(
-  Schema.fromJsonString(StartupLockPayload),
-);
-
-const parseLockPid = (raw: string): number | null => {
-  const decoded = decodeStartupLockPayload(raw);
-  return Option.isSome(decoded) ? decoded.value.pid : null;
-};
-
-export const acquireLocalServerStartLock = (): Effect.Effect<
-  LocalServerStartLock,
-  Error,
-  FileSystem.FileSystem | Path.Path
-> =>
-  Effect.gen(function* () {
-    const fs = yield* FileSystem.FileSystem;
-    const path = yield* Path.Path;
-    yield* fs.makeDirectory(serverControlDir(path), { recursive: true });
-
-    const lockPath = localServerStartLockPath(path);
-    const lockPayload = `${JSON.stringify({ pid: process.pid, startedAt: new Date().toISOString() }, null, 2)}\n`;
-
-    const tryAcquire = () =>
-      fs.writeFileString(lockPath, lockPayload, { flag: "wx" }).pipe(
-        Effect.as(true),
-        Effect.catchCause(() => Effect.succeed(false)),
-      );
-
-    if (yield* tryAcquire()) return { path: lockPath };
-
-    const existingRaw = yield* fs
-      .readFileString(lockPath)
-      .pipe(Effect.catchCause(() => Effect.succeed(null)));
-    if (existingRaw !== null) {
-      const existingPid = parseLockPid(existingRaw);
-      if (existingPid !== null && !isPidAlive(existingPid)) {
-        yield* fs.remove(lockPath, { force: true });
-        if (yield* tryAcquire()) return { path: lockPath };
-      }
-    }
-
-    return yield* Effect.fail(
-      new Error("Another local Executor server startup is already in progress."),
-    );
-  });
-
-export const releaseLocalServerStartLock = (
-  lock: LocalServerStartLock,
-): Effect.Effect<void, PlatformError, FileSystem.FileSystem | Path.Path> =>
-  Effect.gen(function* () {
-    const fs = yield* FileSystem.FileSystem;
-    yield* fs.remove(lock.path, { force: true });
-  });