From e7ae306aa18b5b25e96e009b830628fa977fa792 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 14 May 2026 08:00:36 +0100 Subject: [PATCH] refactor(auth): use fs-safe stale lock recovery --- package.json | 5 ++- pnpm-lock.yaml | 19 +++++++--- pnpm-workspace.yaml | 2 +- src/infra/stale-lock-file.ts | 68 ------------------------------------ src/plugin-sdk/file-lock.ts | 57 ++++++++++-------------------- 5 files changed, 38 insertions(+), 113 deletions(-) diff --git a/package.json b/package.json index 2eac6dec3bb..1a5016743ee 100644 --- a/package.json +++ b/package.json @@ -1766,7 +1766,10 @@ "@lydell/node-pty": "1.2.0-beta.12", "@modelcontextprotocol/sdk": "1.29.0", "@mozilla/readability": "0.6.0", - "@openclaw/fs-safe": "0.2.3", + "@openclaw/fs-safe": "0.2.4", + "@slack/bolt": "4.7.2", + "@slack/types": "2.21.1", + "@slack/web-api": "7.15.2", "ajv": "8.20.0", "chalk": "5.6.2", "chokidar": "5.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6a92da1a111..1db302a7f06 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -84,8 +84,17 @@ importers: specifier: 0.6.0 version: 0.6.0 '@openclaw/fs-safe': - specifier: 0.2.3 - version: 0.2.3 + specifier: 0.2.4 + version: 0.2.4 + '@slack/bolt': + specifier: 4.7.2 + version: 4.7.2(@types/express@5.0.6) + '@slack/types': + specifier: 2.21.1 + version: 2.21.1 + '@slack/web-api': + specifier: 7.15.2 + version: 7.15.2 ajv: specifier: 8.20.0 version: 8.20.0 @@ -3325,8 +3334,8 @@ packages: cpu: [x64] os: [win32] - '@openclaw/fs-safe@0.2.3': - resolution: {integrity: sha512-O8AJ/ZiLbBQvpxYyXrZuyIFFUG2N2Oz6uhle5Gn1gVGkgT+Qmx7O1A9tbE2pTZs2Cyk75A31H0F8w34CL2X6gg==} + '@openclaw/fs-safe@0.2.4': + resolution: {integrity: sha512-Fo3WTQhxu0asD/rZqIKBqhX6fuZfjyHxSW5yTKfcRx+D9BRAcz0AGoVh+3ur/4XRvZkvsh3Ud8XTw006yRYLgg==} engines: {node: '>=20.11'} '@opentelemetry/api-logs@0.217.0': @@ -10004,7 +10013,7 @@ snapshots: '@openai/codex@0.130.0-win32-x64': optional: true - '@openclaw/fs-safe@0.2.3': + '@openclaw/fs-safe@0.2.4': optionalDependencies: jszip: 3.10.1 tar: 7.5.15 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index bdb0db8bc91..501ec05cc93 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -7,7 +7,7 @@ packages: minimumReleaseAge: 2880 minimumReleaseAgeExclude: - - "@openclaw/fs-safe@0.2.3" + - "@openclaw/fs-safe@0.2.4" - "acpx" - "tokenjuice" - "@agentclientprotocol/sdk" diff --git a/src/infra/stale-lock-file.ts b/src/infra/stale-lock-file.ts index d59f3025309..79ea94cc3e1 100644 --- a/src/infra/stale-lock-file.ts +++ b/src/infra/stale-lock-file.ts @@ -1,11 +1,5 @@ -import fs from "node:fs/promises"; import { isPidDefinitelyDead as defaultIsPidDefinitelyDead } from "../shared/pid-alive.js"; -export type LockFileSnapshot = { - raw: string; - payload: Record | null; -}; - export type LockFileOwnerPayload = { pid?: number; createdAt?: string; @@ -23,31 +17,6 @@ export function readLockFileOwnerPayload( }; } -export async function readLockFileSnapshot(lockPath: string): Promise { - let raw: string; - try { - raw = await fs.readFile(lockPath, "utf8"); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "ENOENT") { - return null; - } - throw err; - } - - try { - const parsed = JSON.parse(raw) as unknown; - return { - raw, - payload: - parsed && typeof parsed === "object" && !Array.isArray(parsed) - ? (parsed as Record) - : null, - }; - } catch { - return { raw, payload: null }; - } -} - export function shouldRemoveDeadOwnerOrExpiredLock(params: { payload: Record | null; staleMs: number; @@ -64,40 +33,3 @@ export function shouldRemoveDeadOwnerOrExpiredLock(params: { } return false; } - -export async function removeLockFileIfSnapshotMatches(params: { - lockPath: string; - snapshot: LockFileSnapshot; -}): Promise { - const current = await readLockFileSnapshot(params.lockPath); - if (!current) { - return true; - } - if (current.raw !== params.snapshot.raw) { - return false; - } - - try { - await fs.unlink(params.lockPath); - return true; - } catch (err) { - return (err as NodeJS.ErrnoException).code === "ENOENT"; - } -} - -export async function removeReportedStaleLockIfStillStale(params: { - lockPath: string; - shouldRemove: (snapshot: LockFileSnapshot) => boolean | Promise; -}): Promise { - const snapshot = await readLockFileSnapshot(params.lockPath); - if (!snapshot) { - return true; - } - if (!(await params.shouldRemove(snapshot))) { - return false; - } - return await removeLockFileIfSnapshotMatches({ - lockPath: params.lockPath, - snapshot, - }); -} diff --git a/src/plugin-sdk/file-lock.ts b/src/plugin-sdk/file-lock.ts index 11c404a23f4..06b5208dd62 100644 --- a/src/plugin-sdk/file-lock.ts +++ b/src/plugin-sdk/file-lock.ts @@ -4,10 +4,7 @@ import { drainFileLockManagerForTest, resetFileLockManagerForTest, } from "@openclaw/fs-safe/file-lock"; -import { - removeReportedStaleLockIfStillStale, - shouldRemoveDeadOwnerOrExpiredLock, -} from "../infra/stale-lock-file.js"; +import { shouldRemoveDeadOwnerOrExpiredLock } from "../infra/stale-lock-file.js"; export type FileLockOptions = { retries: { @@ -53,10 +50,6 @@ async function shouldReclaimPluginLock(params: { }); } -function isFileLockError(error: unknown, code: string): boolean { - return (error as { code?: unknown } | null)?.code === code; -} - function normalizeLockError(err: unknown): never { if ((err as { code?: unknown }).code === FILE_LOCK_TIMEOUT_ERROR_CODE) { throw Object.assign(new Error((err as Error).message), { @@ -86,36 +79,24 @@ export async function acquireFileLock( filePath: string, options: FileLockOptions, ): Promise { - while (true) { - try { - const lock = await acquireFsSafeFileLock(filePath, { - managerKey: FILE_LOCK_MANAGER_KEY, - staleMs: options.stale, - retry: options.retries, - allowReentrant: true, - payload: () => ({ pid: process.pid, createdAt: new Date().toISOString() }), - shouldReclaim: shouldReclaimPluginLock, - }); - return { lockPath: lock.lockPath, release: lock.release }; - } catch (err) { - if (isFileLockError(err, FILE_LOCK_STALE_ERROR_CODE)) { - const lockPath = (err as { lockPath?: string }).lockPath; - if ( - lockPath && - (await removeReportedStaleLockIfStillStale({ - lockPath, - shouldRemove: (snapshot) => - shouldRemoveDeadOwnerOrExpiredLock({ - payload: snapshot.payload, - staleMs: options.stale, - }), - })) - ) { - continue; - } - } - return normalizeLockError(err); - } + try { + const lock = await acquireFsSafeFileLock(filePath, { + managerKey: FILE_LOCK_MANAGER_KEY, + staleMs: options.stale, + retry: options.retries, + staleRecovery: "remove-if-unchanged", + allowReentrant: true, + payload: () => ({ pid: process.pid, createdAt: new Date().toISOString() }), + shouldReclaim: shouldReclaimPluginLock, + shouldRemoveStaleLock: (snapshot) => + shouldRemoveDeadOwnerOrExpiredLock({ + payload: snapshot.payload, + staleMs: options.stale, + }), + }); + return { lockPath: lock.lockPath, release: lock.release }; + } catch (err) { + return normalizeLockError(err); } }