fix(db): recover WASM SQLite locks via PID sentinel, not a 60s age window (#1261)

## Summary

Follow-up to #1260 addressing a post-merge finding from **Cursor
Bugbot** and **sentry-warden**.

On the WASM SQLite fallback (npm package on Node < 22.15),
`node-sqlite3-wasm` locks the DB with a `<db>.lock` directory (mkdir
mutex) and releases it via `rmdir` on close. A CLI process killed by
**SIGINT (Ctrl+C) / SIGTERM** mid-write bypasses the
`process.on('exit')` handler, so the lock dir leaks. The previous
recovery only cleared a leftover lock once it aged past a **60-second**
window — so a quick re-run after Ctrl+C failed with `database is locked`
for up to a minute.

## Fix

PID-owner sentinel for **precise** liveness detection instead of a time
guess:

- On open (WASM), write `<db>.lock.owner` = our PID.
- On the next open, `clearStaleWasmLock` reads it:
- owner **dead** → lock is provably orphaned → cleared **immediately**
(no wait),
- owner **alive** → genuine live lock → left to `busy_timeout` (no
stealing),
  - **no sentinel** → fall back to the age window (older CLI / races).
- On clean close, remove the sentinel.

The sentinel lives **outside** the lock dir — a file *inside* it would
make the driver's `rmdir` unlock fail (verified), defeating the driver's
own cleanup.

### Why not a SIGINT/SIGTERM handler?
A DB-layer signal handler would risk interfering with commands that
legitimately own SIGINT for graceful streaming shutdown (`log list
--follow`, `dashboard view`), which keep the process alive after the
signal. The recovery-side fix has zero interference risk.

## Verification
- New tests: dead-owner lock cleared immediately despite fresh mtime;
live-owner (current PID) lock preserved despite old mtime. 28
sqlite-adapter tests, 296 db tests pass.
- E2E reproduction: a fresh lock dir with a dead-owner sentinel is
cleared on the next run in ~startup time (no 60s stall); previously this
exact case blocked for 60s.
- SEA binary still excludes the WASM driver (0 driver symbols); native
path unchanged.
- npm real-install (no `node_modules`) smoke passes.

Also corrected the now-inaccurate JSDoc that described the (previously
wrong) 'live lock is recently re-touched' premise.

Author: BYK

.lore.md

@@ -483,9 +483,6 @@
 <!-- lore:019f666a-ac76-7581-82f8-8d2af3f3e428 -->
 * **Always perform rigorous, read-only code reviews with specific focus areas**: The user consistently conducts critical, adversarial code reviews in a read-only manner, focusing on specific technical areas. They provide detailed context about the repository, branch, and changes under review, and specify exact focus areas like path correctness, dependency management, build configurations, and security implications. The user maintains strict read-only discipline, prohibiting any modifications to source files, and often works within isolated worktrees. They expect thorough verification of technical changes against documented requirements and prior decisions.
 
-<!-- lore:019e6134-4be2-75d2-8eb3-a69dd468428d -->
-* **Always plan systemic fixes with structured multi-problem breakdowns before implementation**: When the user identifies documentation or tooling issues, they consistently organize them as numbered problems with precise file locations, line numbers, and root causes before any code is written. They expect the assistant to engage at the planning level first — proposing detection strategies, fix approaches, and tradeoffs — and to consolidate related problems (e.g., merging overlapping tasks) rather than treating each in isolation. Plans are written to files and iterated on. Implementation only follows after the plan is agreed upon. The user prefers systemic/automated fixes (e.g., derive patterns from package.json) over one-off patches.
-
 <!-- lore:019f65d7-0856-76f1-8ef3-252b56b66279 -->
 * **Always plans for testing or verification of tasks**: The user consistently implies a need for testing or verification of tasks, as seen in multiple instances where they either directly request tests or imply the necessity for verification. This pattern is observed across various sessions where the user is planning and executing tasks, indicating a preference for ensuring that tasks are validated or tested.
 
@@ -507,6 +504,9 @@
 <!-- lore:019f6b87-5b0e-7186-935e-63eedf17b1ec -->
 * **Always provide comprehensive technical context for implementation decisions**: The user consistently provides detailed technical context when making implementation decisions, including code diffs, test results, build configurations, and dependency analysis. This pattern shows the user expects technical discussions to be grounded in concrete evidence and implementation details, with changes justified by specific requirements like Node version compatibility or WASM driver behavior.
 
+<!-- lore:019f6f45-f55f-7448-bdd5-4b1840e8d3c6 -->
+* **Always provide comprehensive verification artifacts during code reviews**: The user consistently provides detailed verification artifacts when submitting code for review. This includes sharing commit hashes, full code snippets with context, test results across multiple environments, CI status updates, and bot review comments. The user expects reviewers to have complete visibility into the changes and their impact, and proactively shares all relevant information to facilitate thorough review and fast iteration.
+
 <!-- lore:019e610a-fc63-7820-b403-4255d0f0e29e -->
 * **Always read and document full file details before proceeding with analysis or implementation**: When exploring a codebase, the user consistently reads files in full and records comprehensive structured details: exact line counts, all imports, every exported type/interface with their fields, all constants, all function signatures with their logic, and any notable comments or assertions. This applies to both source files and build/tooling scripts. The user expects the assistant to capture and reference these details precisely rather than summarizing loosely. When examining related files (e.g., a module and its consumers), the user reads each completely before drawing conclusions. This pattern applies during architecture exploration, feature planning, and documentation generation tasks.
 
@@ -570,6 +570,9 @@
 <!-- lore:019f60f6-ec63-7db9-b92a-fd75ecf48e2b -->
 * **Always Verify and Validate Code Correctness**: The user consistently verifies and validates code correctness across multiple sessions. This includes checking the correctness of functions, ensuring proper scoping of catch blocks, verifying variable destructuring, and confirming the integrity of specific code elements. The user also tends to ask questions about code behavior, compatibility, and potential issues, demonstrating a thorough approach to code review and validation.
 
+<!-- lore:019f6fa8-3cae-7682-afa0-80d90a694728 -->
+* **Always verify bot findings and test coverage before merging**: The user consistently demonstrates a pattern of thorough verification before merging changes. They always review bot findings (sentry-warden, Cursor Bugbot, etc.) in detail, share relevant code snippets to verify findings against current code, and run comprehensive tests including edge cases. The user expects to see test results for all scenarios, especially for database operations and signal handling. They also verify that fixes address the root cause and don't introduce new issues.
+
 <!-- lore:019e6107-5329-700f-b5b8-e7fd5804d036 -->
 * **Always verify code claims against actual file contents before accepting them as true**: When evaluating PRs, documentation, or assertions about code behavior, the user systematically cross-checks every claim against the actual source files at specific line numbers. They expect the assistant to read the real files, confirm exact line locations, quote the relevant code/comments, and flag discrepancies between what is claimed and what the code actually does. The user marks confirmed findings with 🟡 (verified) and actionable directives with 🔴 (user assertion/directive). Never accept a PR description or assertion at face value — always ground-truth it against the codebase with precise line references.
 

src/lib/binary.ts

@@ -25,7 +25,7 @@ import {
 } from "./custom-ca.js";
 import { stringifyUnknown, UpgradeError } from "./errors.js";
 import { logger } from "./logger.js";
-
+import { isProcessRunning } from "./process-utils.js";
 /** Known directories where the curl installer may place the binary */
 export const KNOWN_CURL_DIRS = [".local/bin", "bin", ".sentry/bin"];
 
@@ -360,27 +360,6 @@ export function cleanupOldBinary(oldPath: string): void {
 
 // Lock Management
 
-/**
- * Check if a process with the given PID is still running.
- *
- * On Unix, process.kill(pid, 0) throws:
- * - ESRCH: Process does not exist (not running)
- * - EPERM: Process exists but we lack permission to signal it (IS running)
- */
-export function isProcessRunning(pid: number): boolean {
-  try {
-    process.kill(pid, 0); // Signal 0 just checks if process exists
-    return true;
-  } catch (error) {
-    // EPERM means process exists but we can't signal it (different user)
-    if ((error as NodeJS.ErrnoException).code === "EPERM") {
-      return true;
-    }
-    // ESRCH or other errors mean process is not running
-    return false;
-  }
-}
-
 /**
  * Acquire an exclusive lock for binary installation/upgrade.
  * Uses atomic file creation with 'wx' flag to prevent race conditions.

src/lib/db/index.ts

@@ -44,8 +44,10 @@ let dbOpenedPath: string | null = null;
  * wrapper finalizes cursors so this happens reliably. Explicitly closing on
  * a normal `exit` is a proactive backstop that shrinks the window in which a
  * lock could linger; it does NOT fire on SIGKILL/SIGINT, so it is not the
- * primary guarantee. Crash-orphaned locks are recovered at open time by
- * {@link clearStaleWasmLock}. Harmless (and cheap) for the native driver too.
+ * primary guarantee. A lock orphaned by a signal-killed process is recovered
+ * at the next open by `clearStaleWasmLock`, which uses the PID-owner sentinel
+ * to clear it immediately once the owner is gone. Harmless (and cheap) for the
+ * native driver too.
  */
 let exitHandlerRegistered = false;
 

src/lib/db/sqlite.ts

@@ -20,9 +20,16 @@
  * return-value differences that are reconciled here.
  */
 
-import { rmdirSync, statSync } from "node:fs";
+import {
+  readFileSync,
+  rmdirSync,
+  rmSync,
+  statSync,
+  writeFileSync,
+} from "node:fs";
 import { createRequire } from "node:module";
 import { logger } from "../logger.js";
+import { isProcessRunning } from "../process-utils.js";
 
 const _require = createRequire(import.meta.url);
 
@@ -257,53 +264,104 @@ function resolveDriver(): ResolvedDriver {
 }
 
 /**
- * Age (ms) beyond which a `<db>.lock` directory is considered stale.
+ * Fallback age (ms) beyond which a `<db>.lock` directory with no usable PID
+ * sentinel is treated as stale.
  *
- * The WASM driver holds the lock only briefly (during a write transaction's
- * lock escalation) and downgrades between operations, so a live lock's
- * directory is re-created and thus recently modified. A directory older than
- * this window is therefore almost certainly orphaned by a dead process. The
- * value is deliberately generous (well beyond any single write) to avoid
- * ever racing a genuinely concurrent writer.
+ * The WASM driver acquires the lock (`mkdir`) for the duration of a write and
+ * releases it (`rmdir`) when SQLite drops to lock level NONE; while held, the
+ * directory's mtime is frozen at acquisition time (the driver never re-touches
+ * it). A held lock therefore ages, so a pure time threshold cannot by itself
+ * distinguish "long-running live writer" from "orphaned by a dead process" —
+ * that is what the PID sentinel is for. This age check is only the fallback
+ * for locks with no readable sentinel (e.g. left by an older CLI version); it
+ * is kept generously large so it never races a plausibly-live writer.
  */
 const STALE_LOCK_MAX_AGE_MS = 60_000;
 
+/**
+ * Minimum age (ms) before a lock may be removed *at all*, even when the PID
+ * sentinel says its owner is dead.
+ *
+ * The sentinel is a single shared file, so it can be stale: a leftover
+ * `.lock.owner` naming a dead PID may sit next to a lock that a *different*,
+ * still-live process (e.g. an older CLI that doesn't write sentinels) just
+ * acquired. Refusing to delete any lock younger than this floor guarantees we
+ * never steal a freshly-acquired live lock on the strength of a stale
+ * sentinel. It is comfortably larger than a single write's lock hold (sub-ms
+ * to a few ms) but small enough that Ctrl+C recovery is near-instant instead
+ * of the old 60s wait.
+ */
+const LOCK_SAFETY_FLOOR_MS = 3000;
+
+/**
+ * Sibling file recording the PID of the process that most recently opened the
+ * DB on the WASM path. Lets a later invocation test whether a leftover
+ * `<db>.lock` belongs to a still-running process or an orphan. Kept *outside*
+ * the lock directory itself: a file inside it would make the driver's
+ * `rmdir`-based unlock fail, defeating the driver's own cleanup.
+ */
+function lockOwnerPath(dbPath: string): string {
+  return `${dbPath}.lock.owner`;
+}
+
 /**
  * Remove a stale lock directory left by a previous `node-sqlite3-wasm`
  * process.
  *
  * The WASM driver implements SQLite file locking by `mkdir("<db>.lock")`
  * (an atomic mutex) and releasing it with `rmdir` only when SQLite lowers
- * the lock to NONE. If a process exits while still holding any lock level
- * — including a normal exit where the connection isn't explicitly downgraded
- * — the empty directory is left behind, and the next invocation's `mkdir`
- * fails with EEXIST, surfacing as "database is locked" forever.
+ * the lock to NONE. If a process is killed (SIGINT/SIGKILL) while holding a
+ * lock — bypassing our normal close — the empty directory is left behind, and
+ * the next invocation's `mkdir` fails with EEXIST, surfacing as "database is
+ * locked".
  *
- * We only remove a lock directory that is *older* than
- * {@link STALE_LOCK_MAX_AGE_MS}, so a lock a concurrently-running CLI just
- * acquired is never deleted out from under it — that live contention is left
- * to SQLite's own `busy_timeout`. A recent lock is kept; only a clearly
- * orphaned one is cleared. Only the empty lock directory is removed (a
- * non-empty dir, never produced by this driver, is left intact by `rmdir`).
+ * Removal is deliberately conservative — it never deletes a lock that could
+ * still be live:
+ *  - A lock younger than {@link LOCK_SAFETY_FLOOR_MS} is always kept, even if a
+ *    (possibly stale) sentinel names a dead PID. This stops a leftover sentinel
+ *    from stealing a lock another process just acquired.
+ *  - Past that floor, a dead-owner sentinel means the lock is provably orphaned
+ *    → clear it immediately (near-instant Ctrl+C recovery).
+ *  - A live-owner sentinel means a genuine live lock → keep it (contention is
+ *    left to SQLite's `busy_timeout`).
+ *  - With no readable sentinel we fall back to the {@link STALE_LOCK_MAX_AGE_MS}
+ *    age heuristic.
  *
- * Best-effort: any failure is swallowed so a permissions issue or a genuine
- * race degrades to the driver's own locking error rather than a crash here.
+ * Only the empty lock directory is removed (`rmdir` leaves a non-empty dir
+ * intact). Best-effort: any failure is swallowed so a permissions issue or a
+ * genuine race degrades to the driver's own locking error rather than a crash.
  */
 function clearStaleWasmLock(dbPath: string): void {
   if (dbPath === ":memory:" || dbPath === "") {
     return;
   }
   const lockDir = `${dbPath}.lock`;
   try {
-    const age = Date.now() - statSync(lockDir).mtimeMs;
-    if (age < STALE_LOCK_MAX_AGE_MS) {
-      // Recent — may belong to a live process. Let busy_timeout handle it.
+    const stat = statSync(lockDir); // throws ENOENT when no lock — common case
+    const age = Date.now() - stat.mtimeMs;
+
+    // Safety floor: too fresh to touch. A lock this young may have just been
+    // acquired by a live peer, and a stale sentinel must not override that.
+    if (age < LOCK_SAFETY_FLOOR_MS) {
       return;
     }
+
+    const owner = readLockOwner(dbPath);
+    if (owner !== null) {
+      if (isProcessRunning(owner)) {
+        // Owner still running: a genuine live lock. Leave it to busy_timeout.
+        return;
+      }
+      // Owner dead and lock past the safety floor: provably orphaned.
+    } else if (age < STALE_LOCK_MAX_AGE_MS) {
+      // No sentinel to prove death; not old enough for the age fallback.
+      return;
+    }
+
     // rmdirSync only removes empty directories, so this can never delete a
     // lock actively held with contents (this driver's locks are always empty).
     rmdirSync(lockDir);
-    log.debug(`Cleared stale WASM SQLite lock (age ${age}ms): ${lockDir}`);
+    log.debug(`Cleared orphaned WASM SQLite lock: ${lockDir}`);
   } catch (error) {
     const code = (error as NodeJS.ErrnoException).code;
     // ENOENT: no stale lock (the common case). Anything else is logged.
@@ -313,6 +371,47 @@ function clearStaleWasmLock(dbPath: string): void {
   }
 }
 
+/**
+ * Record the current process as the WASM lock owner (best-effort).
+ * Written on open so a later invocation can test our liveness if we're killed
+ * mid-write before {@link removeLockOwner} runs.
+ */
+function writeLockOwner(dbPath: string): void {
+  if (dbPath === ":memory:" || dbPath === "") {
+    return;
+  }
+  try {
+    writeFileSync(lockOwnerPath(dbPath), String(process.pid));
+  } catch (error) {
+    log.debug("Could not write WASM lock owner sentinel", error);
+  }
+}
+
+/** Read the recorded owner PID, or null if absent/unreadable. */
+function readLockOwner(dbPath: string): number | null {
+  try {
+    const pid = Number.parseInt(
+      readFileSync(lockOwnerPath(dbPath), "utf8"),
+      10
+    );
+    return Number.isInteger(pid) && pid > 0 ? pid : null;
+  } catch {
+    return null;
+  }
+}
+
+/** Remove our owner sentinel on clean close (best-effort). */
+function removeLockOwner(dbPath: string): void {
+  if (dbPath === ":memory:" || dbPath === "") {
+    return;
+  }
+  try {
+    rmSync(lockOwnerPath(dbPath), { force: true });
+  } catch (error) {
+    log.debug("Could not remove WASM lock owner sentinel", error);
+  }
+}
+
 /**
  * SQLite database wrapper.
  *
@@ -328,13 +427,22 @@ export class Database {
   /** Which backing driver this instance uses (for param normalisation). */
   private readonly kind: DriverKind;
 
+  /** On-disk path (for WASM lock-owner sentinel cleanup on close). */
+  private readonly path: string;
+
   constructor(path: string) {
     const { Ctor, kind } = resolveDriver();
     if (kind === "wasm") {
       clearStaleWasmLock(path);
     }
     this.db = new Ctor(path);
     this.kind = kind;
+    this.path = path;
+    if (kind === "wasm") {
+      // Record ourselves so a later invocation can tell whether a leftover
+      // lock (e.g. after we're SIGINT-killed mid-write) is orphaned or live.
+      writeLockOwner(path);
+    }
   }
 
   /**
@@ -369,11 +477,15 @@ export class Database {
    * remove the lock directory ourselves here: `<db>.lock` is a path-keyed
    * cross-process mutex, so a stray `rmdir` could delete a concurrently-
    * running CLI's live lock and allow two writers at once (corruption). Stale
-   * locks left by a crashed process are handled conservatively at open time by
-   * {@link clearStaleWasmLock}.
+   * locks left by a crashed process are handled at open time by
+   * {@link clearStaleWasmLock}. We do clear our own owner sentinel so it can't
+   * linger and be misread by a later invocation.
    */
   close(): void {
     this.db.close();
+    if (this.kind === "wasm") {
+      removeLockOwner(this.path);
+    }
   }
 
   /**

src/lib/process-utils.ts

@@ -0,0 +1,31 @@
+/**
+ * Small OS-process helpers shared across modules.
+ *
+ * Kept as a dependency-free leaf module so lean, hot-path callers (e.g. the
+ * SQLite adapter, loaded on every command) can reuse process liveness checks
+ * without pulling in the heavier graphs of modules like `binary.ts`.
+ */
+
+/**
+ * Check if a process with the given PID is still running.
+ *
+ * On Unix, `process.kill(pid, 0)` sends no signal but performs error checking:
+ * - ESRCH: process does not exist (not running).
+ * - EPERM: process exists but we lack permission to signal it (IS running).
+ *
+ * @param pid Process ID to probe.
+ * @returns `true` if a process with this PID currently exists.
+ */
+export function isProcessRunning(pid: number): boolean {
+  try {
+    process.kill(pid, 0); // Signal 0 just checks if the process exists.
+    return true;
+  } catch (error) {
+    // EPERM means the process exists but we can't signal it (different user).
+    if ((error as NodeJS.ErrnoException).code === "EPERM") {
+      return true;
+    }
+    // ESRCH or any other error means the process is not running.
+    return false;
+  }
+}