From 35e3eff549dacf5f9fcdcd983de1d14857fd7229 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 12:38:00 +0100 Subject: [PATCH] fix(agents): propagate auth lock release failures --- CHANGELOG.md | 1 + src/agents/sessions/auth-storage.test.ts | 15 ++++++- src/agents/sessions/auth-storage.ts | 55 ++++++++---------------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b1f105fbadd..8f8db66555c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- **Agent auth storage locks:** surface normal release failures while avoiding redundant release attempts after `proper-lockfile` reports a compromised lock. - **Paired-node session catalogs:** authorize bundled Anthropic and Codex catalog requests to invoke their read-only node commands from Control UI read flows, restoring remote Claude/Codex rows and terminal resume availability. Fixes #107406. - **Sandbox recreate confirmation:** treat Clack cancellation as a decline so Ctrl-C cannot proceed with container removal. - **Microsoft Teams HTML text:** decode HTML5 entities consistently in quoted and Graph-fetched messages while preserving literal escaped entity text. diff --git a/src/agents/sessions/auth-storage.test.ts b/src/agents/sessions/auth-storage.test.ts index dc42b5114da5..38ebed7dc49b 100644 --- a/src/agents/sessions/auth-storage.test.ts +++ b/src/agents/sessions/auth-storage.test.ts @@ -2,6 +2,7 @@ // contracts for agent model authentication. import { tmpdir } from "node:os"; import { join } from "node:path"; +import lockfile from "proper-lockfile"; import { afterEach, describe, expect, it, vi } from "vitest"; // auth-storage.ts persists via the named import `writeFileSync` from node:fs, @@ -35,12 +36,13 @@ vi.mock("node:fs", async (importOriginal) => { }); const fs = await import("node:fs"); -const { AuthStorage } = await import("./auth-storage.js"); +const { AuthStorage, FileAuthStorageBackend } = await import("./auth-storage.js"); -describe("auth-storage survives an interrupted write during persist (atomic write)", () => { +describe("file auth storage", () => { let tmpDir: string | undefined; afterEach(() => { + vi.restoreAllMocks(); writeFailHook.fn = undefined; if (tmpDir) { fs.rmSync(tmpDir, { recursive: true, force: true }); @@ -116,4 +118,13 @@ describe("auth-storage survives an interrupted write during persist (atomic writ expect(fs.statSync(tmpDir).mode & 0o777).toBe(0o755); expect(fs.statSync(authPath).mode & 0o777).toBe(0o600); }); + + it("propagates async lock release failures", async () => { + tmpDir = fs.mkdtempSync(join(tmpdir(), "auth-releasefail-")); + const error = new Error("simulated lock release failure"); + vi.spyOn(lockfile, "lock").mockResolvedValueOnce(() => Promise.reject(error)); + const storage = new FileAuthStorageBackend(join(tmpDir, "auth.json")); + + await expect(storage.withLockAsync(async () => ({ result: undefined }))).rejects.toBe(error); + }); }); diff --git a/src/agents/sessions/auth-storage.ts b/src/agents/sessions/auth-storage.ts index b2c01f6dbaad..3eb7b88c5b62 100644 --- a/src/agents/sessions/auth-storage.ts +++ b/src/agents/sessions/auth-storage.ts @@ -98,9 +98,8 @@ export class FileAuthStorageBackend implements AuthStorageBackend { this.ensureParentDir(); this.ensureFileExists(); - let release: (() => void) | undefined; + const release = acquireLockSyncWithRetry(this.authPath); try { - release = acquireLockSyncWithRetry(this.authPath); const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined; const { result, next } = fn(current); if (next !== undefined) { @@ -108,9 +107,7 @@ export class FileAuthStorageBackend implements AuthStorageBackend { } return result; } finally { - if (release) { - release(); - } + release(); } } @@ -118,47 +115,31 @@ export class FileAuthStorageBackend implements AuthStorageBackend { this.ensureParentDir(); this.ensureFileExists(); - let release: (() => Promise) | undefined; - let lockCompromised = false; let lockCompromisedError: Error | undefined; - const throwIfCompromised = () => { - if (lockCompromised) { - throw lockCompromisedError ?? new Error("Auth storage lock was compromised"); - } - }; - + const release = await lockfile.lock(this.authPath, { + retries: { + minTimeout: 100, + maxTimeout: 10000, + randomize: true, + }, + stale: 30000, + onCompromised: (err) => { + lockCompromisedError = err; + }, + }); try { - release = await lockfile.lock(this.authPath, { - retries: { - retries: 10, - factor: 2, - minTimeout: 100, - maxTimeout: 10000, - randomize: true, - }, - stale: 30000, - onCompromised: (err) => { - lockCompromised = true; - lockCompromisedError = err; - }, - }); - - throwIfCompromised(); const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined; const { result, next } = await fn(current); - throwIfCompromised(); + if (lockCompromisedError) { + throw lockCompromisedError; + } if (next !== undefined) { this.replaceAuthFileAtomic(next); } - throwIfCompromised(); return result; } finally { - if (release) { - try { - await release(); - } catch { - // Ignore unlock errors when lock is compromised. - } + if (!lockCompromisedError) { + await release(); } } }