From e7fd4759268c3221e97e7d5c7b1be75ed692eb60 Mon Sep 17 00:00:00 2001
From: Agnik47 <140933190+Agnik47@users.noreply.github.com>
Date: Mon, 7 Sep 2026 20:20:36 +0530
Subject: [PATCH] fix(site-memory): stop a stale file lock from being stolen
 while the owner is still alive

breakIfAbandoned() broke a lock whenever it was older than staleMs,
regardless of whether the owning process was still running its critical
section. A same-host owner confirmed alive by process.kill(pid, 0) is now
decisive: staleness alone never steals its lock, only a confirmed-dead
owner or an unverifiable (cross-host) owner does.

Fixes #432

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018qEzPWwgbBFci2RcjPAhMn
---
 src/site-memory/file-lock.test.ts | 34 +++++++++++++++++++++++++++++++
 src/site-memory/file-lock.ts      | 20 ++++++++++++------
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/src/site-memory/file-lock.test.ts b/src/site-memory/file-lock.test.ts
index bfedb2e92..1db040829 100644
--- a/src/site-memory/file-lock.test.ts
+++ b/src/site-memory/file-lock.test.ts
@@ -72,6 +72,40 @@ describe('site memory file lock', () => {
       .resolves.toBe('written');
   });
 
+  it('does not steal a stale lock from a same-host owner that is still alive', async () => {
+    const target = await tempTarget();
+    const lockPath = lockPathFor(target);
+    await writeFile(lockPath, `${JSON.stringify({ pid: process.pid, host: hostname(), token: 'still-working' })}\n`);
+    const past = new Date(Date.now() - 60_000);
+    await utimes(lockPath, past, past);
+
+    await expect(withFileLock(target, async () => 'written', { staleMs: 10_000, timeoutMs: 50 }))
+      .rejects.toMatchObject({ code: 'SITE_MEMORY_BUSY' });
+
+    await expect(readFile(lockPath, 'utf8')).resolves.toContain('still-working');
+  });
+
+  it('does not let two critical sections overlap when one runs longer than staleMs', async () => {
+    const target = await tempTarget();
+    let insideFirst = false;
+    let overlapped = false;
+
+    const first = withFileLock(target, async () => {
+      insideFirst = true;
+      await new Promise((resolve) => { setTimeout(resolve, 120); });
+      insideFirst = false;
+    }, { staleMs: 30, timeoutMs: 5_000 });
+
+    await new Promise((resolve) => { setTimeout(resolve, 60); });
+
+    const second = withFileLock(target, async () => {
+      if (insideFirst) overlapped = true;
+    }, { staleMs: 30, timeoutMs: 5_000 });
+
+    await Promise.all([first, second]);
+    expect(overlapped).toBe(false);
+  });
+
   it('reports a live holder as SITE_MEMORY_BUSY instead of writing anyway', async () => {
     const target = await tempTarget();
     await writeFile(lockPathFor(target), `${JSON.stringify({ pid: process.pid, host: hostname(), token: 'held' })}\n`);
diff --git a/src/site-memory/file-lock.ts b/src/site-memory/file-lock.ts
index 2d1df6a5e..50566d199 100644
--- a/src/site-memory/file-lock.ts
+++ b/src/site-memory/file-lock.ts
@@ -9,11 +9,14 @@
  * marker the filesystem can see: `open(..., 'wx')` creates the lock file only
  * when it does not already exist, atomically, on every platform we support.
  *
- * Abandoned locks never wedge site memory. A lock whose owner process is gone
- * is broken on the next attempt, and any lock older than `staleMs` is broken
- * regardless — the critical section itself is a small read plus a rename, which
- * takes milliseconds. `timeoutMs` is deliberately longer than `staleMs` so an
- * abandoned lock is always broken rather than surfaced to the user as an error.
+ * Abandoned locks never wedge site memory. A lock whose owner process is
+ * confirmed gone is broken on the next attempt. A lock older than `staleMs`
+ * is broken too, but only when we cannot confirm the owner is still alive on
+ * this host — a same-host owner that `process.kill(pid, 0)` finds alive is
+ * still working, no matter how long its critical section has run, and is
+ * never stolen on staleness alone. `timeoutMs` is deliberately longer than
+ * `staleMs` so a genuinely abandoned lock is broken rather than surfaced to
+ * the user as an error.
  */
 import { AsyncLocalStorage } from 'node:async_hooks';
 import { randomUUID } from 'node:crypto';
@@ -116,8 +119,13 @@ async function breakIfAbandoned(lockPath: string, staleMs: number): Promise<bool
   const before = await statOrUndefined(lockPath);
   if (!before) return true;
   const owner = await readOwner(lockPath);
+  const checkable = owner.host === hostname() && isActionablePid(owner.pid);
+  const ownerAlive = checkable && isPidAlive(owner.pid);
+  // A same-host owner we can confirm is still alive is still working, no matter
+  // how long its critical section has run — staleness alone never steals its lock.
+  if (ownerAlive) return false;
+  const ownerGone = checkable && !ownerAlive;
   const expired = Date.now() - before.mtimeMs > staleMs;
-  const ownerGone = owner.host === hostname() && isActionablePid(owner.pid) && !isPidAlive(owner.pid);
   if (!expired && !ownerGone) return false;
 
   const after = await statOrUndefined(lockPath);