fix(lock): scope reentrancy to logical owners

CHANGELOG.md

@@ -32,7 +32,7 @@
 
 ### Compatibility
 
-- Remove the unsound process-scoped `allowReentrant` async file-lock option. Callers that passed it should delete the property; same-process contention now follows the normal retry and timeout policy, while nested same-file `jsonStore` mutations fail immediately instead of deadlocking.
+- Remove the unsound process-scoped `allowReentrant` async file-lock option and replace it with owner-scoped `reentrantOwner` for async and sync locks: only matching, explicitly defined logical owners reuse a canonical in-process lock, releases are reference-counted and idempotent, and different or absent owners contend normally. Callers that passed the boolean must either remove it or migrate intentional nesting to a per-operation owner key; `jsonStore` remains ownerless and rejects nested same-file mutations immediately.
 - Remove the persistent Python helper and its `pythonPath` configuration. Replace `configureFsSafePython`, `FS_SAFE_PYTHON_MODE`, and the OpenClaw Python aliases with `configureFsSafeNative` and `FS_SAFE_NATIVE_MODE`; 0.5 warns once and maps the former `auto`, `require`, and `off` policies solely as an upgrade bridge for shipped 0.4 consumers.
 - Add `publishFileExclusive({ strategy: "rename-noreplace" })`; this strategy requires the native helper, atomically moves the source, and never replaces an existing destination.
 

docs/migrating-to-0.5.md

@@ -140,11 +140,14 @@ The default directory-error policy remains `throw`. See
 
 - Use `acquireFileLockSync()` only in synchronous boot or migration code; retry
   waits block the thread. Request-serving paths should use `withFileLock()`.
-- Remove `allowReentrant` from async file-lock options. Same-process contention
-  now follows the ordinary retry and timeout policy. Locked and unlocked
-  `jsonStore` mutations serialize by canonical file path; nested same-file
-  mutations from an update callback fail with `store-reentrant-update`, so
-  return the complete value from the outer callback instead.
+- Remove the `allowReentrant` boolean from async file-lock options. If a logical
+  holder intentionally nests acquisition, pass the same operation-scoped
+  `reentrantOwner` string to each acquisition; different or missing owners
+  contend normally. Never replace the boolean with a process-wide constant.
+  Locked and unlocked `jsonStore` mutations serialize by canonical file path
+  and do not opt into lock reentrancy; nested same-file mutations from an update
+  callback fail with `store-reentrant-update`, so return the complete value from
+  the outer callback instead.
 - Use `createSecretFileAtomic()` for first-writer-wins credentials and catch
   `secret-exists`; use `writeSecretFileAtomic()` only when replacement is the
   intended protocol.

docs/sidecar-lock.md

@@ -59,6 +59,7 @@ type FileLockAcquireOptions<TPayload extends Record<string, unknown>> = {
   timeoutMs?: number;                    // overall acquire deadline; default unbounded
   retry?: FileLockRetryOptions;
   staleRecovery?: "fail-closed" | "remove-if-unchanged"; // default "fail-closed"
+  reentrantOwner?: string;               // logical holder identity for owner-scoped nesting
   payload: () => TPayload | Promise<TPayload>;
   shouldReclaim?: (params: {
     lockPath: string;
@@ -95,11 +96,50 @@ type FileLockRetryOptions = {
 result is passed to `shouldReclaim` and `shouldRemoveStaleLock`, allowing PID,
 process-start, argv, or role schemas to remain application-owned.
 
-Async lock acquisition is not reentrant. Another acquisition for the same path,
-including one from the same process and manager, follows the normal retry and
-timeout policy until the current holder releases it. Version 0.5 removes the
-unsound process-scoped `allowReentrant` option; callers that passed it should
-delete the property.
+## Owner-scoped reentrancy
+
+Version 0.5 removes the unsound process-scoped `allowReentrant` boolean and
+replaces it with `reentrantOwner`. When a manager already holds the canonical
+target path, another acquisition reuses that sidecar only when both acquisitions
+provide the same owner string. Each acquisition gets an idempotent release
+handle; the sidecar remains until the last reference is released. A different or
+missing owner waits under the normal contention, retry, and timeout policy. A
+known live in-process holder is never stale-reclaimed by its own manager.
+
+This supports logical session writers that may reach one file through real and
+symlinked parent paths:
+
+```ts
+const managerKey = "session-write-locks";
+const reentrantOwner = `session:${sessionId}:operation:${operationId}`;
+
+const outer = await acquireFileLock(realSessionPath, {
+  managerKey,
+  reentrantOwner,
+  staleMs: 60_000,
+  payload: () => ({ pid: process.pid, operationId }),
+});
+const nested = await acquireFileLock(symlinkedSessionPath, {
+  managerKey,
+  reentrantOwner,
+  staleMs: 60_000,
+  payload: () => ({ pid: process.pid, operationId }),
+});
+
+await nested.release(); // sidecar remains for outer
+await outer.release();  // final reference removes it
+```
+
+The manager domain and canonical target path are part of the identity, so
+aliased paths must use the same `managerKey`. The owner key must identify one
+logical holder or call chain. **Never use a process-wide or other shared constant
+for unrelated tasks**: doing so would admit concurrent work to the same critical
+section and recreate the lost-update bug that removed `allowReentrant`.
+
+Omit `reentrantOwner` for ordinary acquisitions. `jsonStore` does so and keeps
+its separate canonical-path mutation queue. The synchronous APIs implement the
+same owner/refcount rules; a mismatched synchronous acquisition blocks the
+calling thread according to its retry and timeout options.
 
 Pass `lockRoot` to place sidecar create, read, verification, and removal behind
 an existing `Root` capability. `lockPath` must resolve inside that root.

src/file-lock-sync.ts

@@ -27,6 +27,7 @@ export type FileLockSyncAcquireOptions<TPayload extends Record<string, unknown>>
   timeoutMs?: number;
   retry?: SidecarLockRetryOptions;
   staleRecovery?: SidecarLockStaleRecovery;
+  reentrantOwner?: string;
   payload: () => TPayload;
   shouldReclaim?: (params: {
     lockPath: string;
@@ -51,6 +52,65 @@ export type FileLockSyncHandle = {
   [Symbol.dispose](): void;
 };
 
+type SyncHeldLock = {
+  fd: number;
+  lockPath: string;
+  normalizedTargetPath: string;
+  parsePayload?: (raw: string) => unknown;
+  refCount: number;
+  reentrantOwner?: string;
+  snapshot: SidecarLockSnapshot;
+  timer?: NodeJS.Timeout;
+};
+
+const SYNC_HELD_LOCKS_KEY = Symbol.for("fsSafe.syncSidecarLocks");
+
+function getSyncHeldLocks(): Map<string, SyncHeldLock> {
+  const globalWithState = globalThis as typeof globalThis & {
+    [SYNC_HELD_LOCKS_KEY]?: Map<string, SyncHeldLock>;
+  };
+  if (!globalWithState[SYNC_HELD_LOCKS_KEY]) {
+    globalWithState[SYNC_HELD_LOCKS_KEY] = new Map();
+  }
+  return globalWithState[SYNC_HELD_LOCKS_KEY];
+}
+
+function verifySyncHeldLock(held: SyncHeldLock): boolean {
+  const current = readSidecarLockSnapshotSync(held.lockPath, held.parsePayload);
+  return !!current && sidecarLockSnapshotMatches(current, held.snapshot);
+}
+
+function releaseSyncHeldLock(held: SyncHeldLock): boolean {
+  const heldLocks = getSyncHeldLocks();
+  if (heldLocks.get(held.normalizedTargetPath) !== held) return false;
+  held.refCount -= 1;
+  if (held.refCount > 0) return false;
+  heldLocks.delete(held.normalizedTargetPath);
+  if (held.timer) {
+    clearInterval(held.timer);
+    held.timer = undefined;
+  }
+  fs.closeSync(held.fd);
+  removeSidecarLockIfUnchangedSync(held.lockPath, held.snapshot);
+  return true;
+}
+
+function createSyncHeldLockHandle(held: SyncHeldLock): FileLockSyncHandle {
+  let released = false;
+  const release = () => {
+    if (released) return;
+    released = true;
+    releaseSyncHeldLock(held);
+  };
+  return {
+    lockPath: held.lockPath,
+    normalizedTargetPath: held.normalizedTargetPath,
+    verifyStillHeld: () => verifySyncHeldLock(held),
+    release,
+    [Symbol.dispose]: release,
+  };
+}
+
 function normalizeTargetPath(targetPath: string): string {
   const resolved = path.resolve(targetPath);
   fs.mkdirSync(path.dirname(resolved), { recursive: true });
@@ -93,6 +153,17 @@ export function acquireFileLockSync<TPayload extends Record<string, unknown>>(
 ): FileLockSyncHandle {
   const normalizedTargetPath = normalizeTargetPath(targetPath);
   const lockPath = boundedLockPath(options.lockPath ?? `${normalizedTargetPath}.lock`, options.lockRoot);
+  const heldLocks = getSyncHeldLocks();
+  const held = heldLocks.get(normalizedTargetPath);
+  if (
+    held &&
+    options.reentrantOwner !== undefined &&
+    held.reentrantOwner !== undefined &&
+    options.reentrantOwner === held.reentrantOwner
+  ) {
+    held.refCount += 1;
+    return createSyncHeldLockHandle(held);
+  }
   const staleMs = options.staleMs ?? 30_000;
   const retry = options.retry ?? {};
   const startedAt = Date.now();
@@ -120,38 +191,29 @@ export function acquireFileLockSync<TPayload extends Record<string, unknown>>(
         stat: fs.fstatSync(fd),
         ownershipToken,
       };
-      const heldFd = fd;
-      let released = false;
-      let timer: NodeJS.Timeout | undefined;
-      const verifyStillHeld = () => {
-        const current = readSidecarLockSnapshotSync(lockPath, options.parsePayload);
-        return !!current && sidecarLockSnapshotMatches(current, snapshot);
-      };
-      const release = () => {
-        if (released) return;
-        released = true;
-        if (timer) clearInterval(timer);
-        fs.closeSync(heldFd);
-        fd = undefined;
-        removeSidecarLockIfUnchangedSync(lockPath, snapshot);
+      const createdHeld: SyncHeldLock = {
+        fd,
+        lockPath,
+        normalizedTargetPath,
+        parsePayload: options.parsePayload,
+        refCount: 1,
+        reentrantOwner: options.reentrantOwner,
+        snapshot,
       };
+      heldLocks.set(normalizedTargetPath, createdHeld);
+      const returnedHandle = createSyncHeldLockHandle(createdHeld);
       if (options.onCompromised && (options.compromiseCheckIntervalMs ?? 0) > 0) {
-        timer = setInterval(() => {
-          if (!verifyStillHeld()) {
-            if (timer) clearInterval(timer);
-            timer = undefined;
+        createdHeld.timer = setInterval(() => {
+          if (!returnedHandle.verifyStillHeld()) {
+            if (createdHeld.timer) clearInterval(createdHeld.timer);
+            createdHeld.timer = undefined;
             options.onCompromised?.({ lockPath, normalizedTargetPath });
           }
         }, options.compromiseCheckIntervalMs);
-        timer.unref();
+        createdHeld.timer.unref();
       }
-      return {
-        lockPath,
-        normalizedTargetPath,
-        verifyStillHeld,
-        release,
-        [Symbol.dispose]: release,
-      };
+      fd = undefined;
+      return returnedHandle;
     } catch (error) {
       if (fd !== undefined) {
         const failed = { payload: null, stat: fs.fstatSync(fd) } satisfies SidecarLockSnapshot;
@@ -160,6 +222,20 @@ export function acquireFileLockSync<TPayload extends Record<string, unknown>>(
         removeSidecarLockIfUnchangedSync(lockPath, failed);
       }
       if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
+      if (heldLocks.has(normalizedTargetPath)) {
+        const elapsed = Date.now() - startedAt;
+        const timedOut = options.timeoutMs !== undefined && elapsed >= options.timeoutMs;
+        if (timedOut || (retry.retries !== undefined && attempt >= retry.retries)) {
+          throw Object.assign(new Error(`file lock timeout for ${normalizedTargetPath}`), {
+            code: "file_lock_timeout",
+            lockPath,
+            normalizedTargetPath,
+          });
+        }
+        sleep(computeSidecarLockDelayMs(retry, attempt));
+        attempt += 1;
+        continue;
+      }
       const snapshot = readSidecarLockSnapshotSync(lockPath, options.parsePayload);
       if (!snapshot) continue;
       const nowMs = Date.now();

src/sidecar-lock-handle.ts

@@ -0,0 +1,49 @@
+import type { Root } from "./root-impl.js";
+import {
+  sidecarLockSnapshotStillPresent,
+  type SidecarLockSnapshot,
+} from "./sidecar-lock-reclaim.js";
+import type { SidecarLockHandle } from "./sidecar-lock-types.js";
+
+export function createSidecarLockHandle(params: {
+  lockPath: string;
+  normalizedTargetPath: string;
+  verifyStillHeld: () => Promise<boolean>;
+  release: () => Promise<unknown>;
+}): SidecarLockHandle {
+  let released = false;
+  const release = async (): Promise<void> => {
+    if (released) return;
+    released = true;
+    await params.release();
+  };
+  return {
+    lockPath: params.lockPath,
+    normalizedTargetPath: params.normalizedTargetPath,
+    verifyStillHeld: params.verifyStillHeld,
+    release,
+    [Symbol.asyncDispose]: release,
+  };
+}
+
+export function createHeldSidecarLockHandle(params: {
+  normalizedTargetPath: string;
+  held: {
+    lockPath: string;
+    snapshot: SidecarLockSnapshot;
+    lockRoot?: Root;
+    parsePayload?: (raw: string) => unknown;
+  };
+  release: () => Promise<unknown>;
+}): SidecarLockHandle {
+  return createSidecarLockHandle({
+    lockPath: params.held.lockPath,
+    normalizedTargetPath: params.normalizedTargetPath,
+    verifyStillHeld: async () =>
+      await sidecarLockSnapshotStillPresent(params.held.lockPath, params.held.snapshot, {
+        lockRoot: params.held.lockRoot,
+        parsePayload: params.held.parsePayload,
+      }),
+    release: params.release,
+  });
+}

src/sidecar-lock-types.ts

@@ -23,6 +23,7 @@ export type SidecarLockAcquireOptions<TPayload extends Record<string, unknown>>
   timeoutMs?: number;
   retry?: SidecarLockRetryOptions;
   staleRecovery?: SidecarLockStaleRecovery;
+  reentrantOwner?: string;
   payload: () => TPayload | Promise<TPayload>;
   shouldReclaim?: (params: {
     lockPath: string;