refactor(agents): share sync storage lock retries

steipete · steipete · commit cae29a079203 · 2026-07-13T17:35:29.000-07:00
diff --git a/src/agents/sessions/auth-storage.ts b/src/agents/sessions/auth-storage.ts
@@ -23,6 +23,7 @@ import type {
 } from "../../llm/utils/oauth/types.js";
 import { getAgentDir } from "../config.js";
 import { resolveConfigValue } from "./resolve-config-value.js";
+import { acquireLockSyncWithRetry } from "./storage-lock.js";
 
 export type ApiKeyCredential = {
   type: "api_key";
@@ -93,40 +94,13 @@ export class FileAuthStorageBackend implements AuthStorageBackend {
     });
   }
 
-  private acquireLockSyncWithRetry(path: string): () => void {
-    const maxAttempts = 10;
-    const delayMs = 20;
-    let lastError: unknown;
-
-    for (let attempt = 1; attempt <= maxAttempts; attempt++) {
-      try {
-        return lockfile.lockSync(path, { realpath: false });
-      } catch (error) {
-        const code =
-          typeof error === "object" && error !== null && "code" in error
-            ? String((error as { code?: unknown }).code)
-            : undefined;
-        if (code !== "ELOCKED" || attempt === maxAttempts) {
-          throw error;
-        }
-        lastError = error;
-        const start = Date.now();
-        while (Date.now() - start < delayMs) {
-          // Sleep synchronously to avoid changing callers to async.
-        }
-      }
-    }
-
-    throw (lastError as Error) ?? new Error("Failed to acquire auth storage lock");
-  }
-
   withLock<T>(fn: (current: string | undefined) => LockResult<T>): T {
     this.ensureParentDir();
     this.ensureFileExists();
 
     let release: (() => void) | undefined;
     try {
-      release = this.acquireLockSyncWithRetry(this.authPath);
+      release = acquireLockSyncWithRetry(this.authPath);
       const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined;
       const { result, next } = fn(current);
       if (next !== undefined) {
diff --git a/src/agents/sessions/settings-manager.ts b/src/agents/sessions/settings-manager.ts
@@ -7,11 +7,11 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
 import { homedir } from "node:os";
 import { dirname, join } from "node:path";
 import { resolveIntegerOption } from "@openclaw/normalization-core/number-coercion";
-import lockfile from "proper-lockfile";
 import { mergeDeep } from "../../infra/deep-merge.js";
 import type { Transport } from "../../llm/types.js";
 import { CONFIG_DIR_NAME, getAgentDir } from "../config.js";
 import { DEFAULT_HTTP_IDLE_TIMEOUT_MS, parseHttpIdleTimeoutMs } from "./http-dispatcher.js";
+import { acquireLockSyncWithRetry } from "./storage-lock.js";
 
 interface CompactionSettings {
   enabled?: boolean; // default: true
@@ -147,33 +147,6 @@ export class FileSettingsStorage implements SettingsStorage {
     this.projectSettingsPath = join(cwd, CONFIG_DIR_NAME, "settings.json");
   }
 
-  private acquireLockSyncWithRetry(path: string): () => void {
-    const maxAttempts = 10;
-    const delayMs = 20;
-    let lastError: unknown;
-
-    for (let attempt = 1; attempt <= maxAttempts; attempt++) {
-      try {
-        return lockfile.lockSync(path, { realpath: false });
-      } catch (error) {
-        const code =
-          typeof error === "object" && error !== null && "code" in error
-            ? String((error as { code?: unknown }).code)
-            : undefined;
-        if (code !== "ELOCKED" || attempt === maxAttempts) {
-          throw error;
-        }
-        lastError = error;
-        const start = Date.now();
-        while (Date.now() - start < delayMs) {
-          // Sleep synchronously to avoid changing callers to async.
-        }
-      }
-    }
-
-    throw (lastError as Error) ?? new Error("Failed to acquire settings lock");
-  }
-
   withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void {
     const path = scope === "global" ? this.globalSettingsPath : this.projectSettingsPath;
     const dir = dirname(path);
@@ -183,7 +156,7 @@ export class FileSettingsStorage implements SettingsStorage {
       // Only create directory and lock if file exists or we need to write
       const fileExists = existsSync(path);
       if (fileExists) {
-        release = this.acquireLockSyncWithRetry(path);
+        release = acquireLockSyncWithRetry(path);
       }
       const current = fileExists ? readFileSync(path, "utf-8") : undefined;
       const next = fn(current);
@@ -193,7 +166,7 @@ export class FileSettingsStorage implements SettingsStorage {
           mkdirSync(dir, { recursive: true });
         }
         if (!release) {
-          release = this.acquireLockSyncWithRetry(path);
+          release = acquireLockSyncWithRetry(path);
         }
         writeFileSync(path, next, "utf-8");
       }
diff --git a/src/agents/sessions/storage-lock.ts b/src/agents/sessions/storage-lock.ts
@@ -0,0 +1,21 @@
+import lockfile from "proper-lockfile";
+
+const MAX_LOCK_ATTEMPTS = 10;
+const LOCK_RETRY_DELAY_MS = 20;
+
+export function acquireLockSyncWithRetry(path: string): () => void {
+  for (let attempt = 1; ; attempt++) {
+    try {
+      return lockfile.lockSync(path, { realpath: false });
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== "ELOCKED" || attempt === MAX_LOCK_ATTEMPTS) {
+        throw error;
+      }
+    }
+
+    const start = Date.now();
+    while (Date.now() - start < LOCK_RETRY_DELAY_MS) {
+      // proper-lockfile rejects sync retries; preserve the bounded sync storage contract.
+    }
+  }
+}
