From b2602cc61d023e94031f3929f97585690ea87d51 Mon Sep 17 00:00:00 2001
From: Dave <dave@sienkowski.com>
Date: Sun, 21 Jun 2026 10:17:28 -0400
Subject: [PATCH] fix: add deadman ceiling to withPlanningLock (M1/M2 R4-FIX
 asymmetry)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The M1/M2 lock fix (4903ee04) was asymmetric: acquireStateLock got a 60s deadman
ceiling (recovers a lock once its age crosses an absolute bound ABOVE the wait
budget) but withPlanningLock did not. The .lock body carries no startTime, so
_planningHolderVerifiedLive can only check pid liveness — it cannot detect pid
reuse. A false-alive holder (original holder crashed, pid recycled by an unrelated
live process) would therefore make withPlanningLock throw on every call with no
self-heal until the reused pid happens to die.

Mirror acquireStateLock: in the EEXIST branch, steal a verified-live holder anyway
once the lock ages past deadmanCeilingMs (60000 > lockTimeout 10000). mtime age is
measured from lock creation, so a stuck lock self-heals on a subsequent call.

Adds a clock+probe-seam regression test (false-alive holder past the ceiling IS
stolen). planning-workspace 13/13, clock-seam 34/34, locking suites green; lint clean.

Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz
---
 src/planning-workspace.cts        | 18 ++++++++++++++++--
 tests/planning-workspace.test.cjs | 22 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/planning-workspace.cts b/src/planning-workspace.cts
index b1ca382f4..a8d11a718 100644
--- a/src/planning-workspace.cts
+++ b/src/planning-workspace.cts
@@ -165,6 +165,12 @@ function withPlanningLock<T>(cwd: string, fn: () => T, clock?: Clock): T {
   if (clock === undefined) clock = realClock;
   const lockPath = path.join(planningDir(cwd), '.lock');
   const lockTimeout = 10000; // 10 seconds
+  // Deadman ceiling (audit M1 / R4-FIX) — set ABOVE lockTimeout so a holder that reads
+  // as alive but is actually a pid-reuse alias (the .lock body has no startTime, so
+  // liveness alone cannot detect reuse) is still recovered once its lock ages past this
+  // absolute ceiling. Without it, a false-alive holder would make withPlanningLock throw
+  // on every call with no self-heal. Mirrors acquireStateLock's deadmanCeilingMs.
+  const deadmanCeilingMs = 60000;
   const start = clock.now();
 
   // Ensure .planning/ exists
@@ -212,9 +218,17 @@ function withPlanningLock<T>(cwd: string, fn: () => T, clock?: Clock): T {
         // A verified-live holder is waited on — never force-stolen — because nuking
         // a slow-but-live writer's lock corrupts the .planning/ critical section.
         try {
-          if (!_planningHolderVerifiedLive(lockPath)) {
+          let stealable = !_planningHolderVerifiedLive(lockPath);
+          if (!stealable) {
+            // Verified-live, but recover anyway once the lock crosses the absolute
+            // deadman ceiling — defeats a pid-reuse false-alive that would otherwise
+            // block forever (R4-FIX; mtime age is from lock creation, not this call).
+            const age = clock.now() - fs.statSync(lockPath).mtimeMs;
+            stealable = age > deadmanCeilingMs;
+          }
+          if (stealable) {
             fs.unlinkSync(lockPath);
-            continue; // dead/garbage holder — retry immediately to grab the freed lock
+            continue; // dead/garbage/expired holder — retry immediately to grab the freed lock
           }
         } catch { continue; }
 
diff --git a/tests/planning-workspace.test.cjs b/tests/planning-workspace.test.cjs
index a996deec2..73ef70898 100644
--- a/tests/planning-workspace.test.cjs
+++ b/tests/planning-workspace.test.cjs
@@ -289,4 +289,26 @@ describe('withPlanningLock PID-liveness staleness + EEXIST safety (audit M1+M2)'
     assert.ok(caught, 'helper must surface a failure rather than silently force-stealing a live lock');
     assert.notStrictEqual(caught.code, 'EEXIST', 'a raw EEXIST must never escape the lock helper (M2)');
   });
+
+  test('R4-FIX: false-alive pid-reuse holder aged past the deadman ceiling IS stolen (self-heal)', () => {
+    const reusedPid = 7373;
+    fs.writeFileSync(lockPath, JSON.stringify({
+      pid: reusedPid,
+      cwd: tmpDir,
+      acquired: new Date().toISOString(),
+    }));
+
+    // Probe says the recorded pid is ALIVE — simulating pid-reuse: the original holder
+    // crashed but its pid was recycled by an unrelated live process. The .lock body has
+    // no startTime, so liveness alone cannot distinguish this from a genuine live holder.
+    planningWorkspaceDirect._setLockProbes({ isPidAlive: (pid) => pid === reusedPid });
+
+    // Lock mtime ≈ now (real); seed the fake clock ABOVE the 60 000 ms deadman ceiling so
+    // age = clock.now() - mtimeMs ≫ ceiling → the lock must be recovered despite "alive".
+    // Without the ceiling, withPlanningLock would throw on every call with no self-heal.
+    const clock = makeFakeClock(Date.now() + 120000);
+    const result = withPlanningLock(tmpDir, () => 'self-healed', clock);
+    assert.strictEqual(result, 'self-healed', 'a false-alive lock past the deadman ceiling must be stolen (no infinite block)');
+    assert.ok(!fs.existsSync(lockPath), 'lock must be released after the critical section completes');
+  });
 });