From 930b3d4934414aba004194f026fdf0ba47439c35 Mon Sep 17 00:00:00 2001 From: jlin53882 Date: Mon, 11 May 2026 09:37:34 +0800 Subject: [PATCH 1/3] feat(store): add validateStoragePathAsync for non-blocking background init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds async version of validateStoragePath (5 sync I/O → async: lstat/realpath/exists/mkdir/access). register() fires off the async validation in setImmediate() so it does not block the event loop during plugin registration. _sync _initPluginState still runs synchronously first so _singletonState is always set before hooks register. _Issue: #795 --- index.ts | 35 ++++++++++++++++++++++++- src/store.ts | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/index.ts b/index.ts index 15d9859e..c57510c1 100644 --- a/index.ts +++ b/index.ts @@ -20,7 +20,7 @@ import { spawn } from "node:child_process"; const isCliMode = () => process.env.OPENCLAW_CLI === "1"; // Import core components -import { MemoryStore, validateStoragePath } from "./src/store.js"; +import { MemoryStore, validateStoragePath, validateStoragePathAsync } from "./src/store.js"; import { createEmbedder, getEffectiveVectorDimensions, @@ -1931,6 +1931,30 @@ interface PluginSingletonState { let _singletonState: PluginSingletonState | null = null; +// Whether the async background validation (validateStoragePathAsync) has completed. +// Used to ensure background validation results are logged before hooks execute. +let _singletonAsyncValidationDone = false; + +function _startAsyncValidation(resolvedDbPath: string, api: OpenClawPluginApi): void { + // Fire-and-forget: run validateStoragePathAsync in setImmediate so the 5 sync + // I/O operations (lstat/realpath/exists/mkdir/access) do NOT block the event loop + // during plugin registration. The sync _initPluginState still runs first so + // _singletonState is always set before hooks register. This function just + // ensures the validation runs without blocking register(). + setImmediate(async () => { + try { + await validateStoragePathAsync(resolvedDbPath); + } catch (err) { + api.logger.warn( + `memory-lancedb-pro: storage path async validation failed — ${String(err)}\n` + + ` The plugin will still attempt to start, but writes may fail.`, + ); + } finally { + _singletonAsyncValidationDone = true; + } + }); +} + function _initPluginState(api: OpenClawPluginApi): PluginSingletonState { const config = parsePluginConfig(api.pluginConfig); const resolvedDbPath = api.resolvePath(config.dbPath || getDefaultDbPath()); @@ -2162,6 +2186,15 @@ const memoryLanceDBProPlugin = { // ======================================================================== _registeredApis.add(api); // claim before init (Phase 2 singleton guard) _registeredApisMap.set(api, true); // dual-track: explicit claim for rollback + + // Start background async validation (fire-and-forget, non-blocking). + // We do this after claiming the API slot so that if the sync _initPluginState + // below throws, the async validation will have already started but its result + // won't matter (plugin failed to start anyway). + const config = parsePluginConfig(api.pluginConfig); + const resolvedDbPath = api.resolvePath(config.dbPath || getDefaultDbPath()); + _startAsyncValidation(resolvedDbPath, api); + let singleton: typeof _singletonState; try { if (!_singletonState) { _singletonState = _initPluginState(api); } diff --git a/src/store.ts b/src/store.ts index 4126a96c..d292de98 100644 --- a/src/store.ts +++ b/src/store.ts @@ -15,6 +15,7 @@ import { statSync, unlinkSync, } from "node:fs"; +import { access, lstat, mkdir, realpath } from "node:fs/promises"; import { dirname, join } from "node:path"; import { buildSmartMetadata, isMemoryActiveAt, parseSmartMetadata, stringifySmartMetadata } from "./smart-metadata.js"; @@ -199,6 +200,79 @@ export function validateStoragePath(dbPath: string): string { return resolvedPath; } +/** + * Async version of validateStoragePath — non-blocking alternative for use in + * background initialization. The sync version (above) is used by _initPluginState + * synchronously; this version is used for fire-and-forget background init. + */ +export async function validateStoragePathAsync(dbPath: string): Promise { + let resolvedPath = dbPath; + + // Resolve symlinks (including dangling symlinks) + try { + const stats = await lstat(dbPath); + if (stats.isSymbolicLink()) { + try { + resolvedPath = await realpath(dbPath); + } catch (err: any) { + throw Object.assign( + new Error( + `dbPath "${dbPath}" is a symlink whose target does not exist.\n` + + ` Fix: Create the target directory, or update the symlink to point to a valid path.\n` + + ` Details: ${err.code || ""} ${err.message}`, + ), + { code: err?.code } + ); + } + } + } catch (err: any) { + // Missing path is OK (it will be created below) + if (err?.code === "ENOENT") { + // no-op + } else if ( + typeof err?.message === "string" && + err.message.includes("symlink whose target does not exist") + ) { + throw err; + } else { + // Other lstat failures — continue with original path + } + } + + // Create directory if it doesn't exist + if (!await pathExistsAsync(resolvedPath)) { + try { + await mkdir(resolvedPath, { recursive: true }); + } catch (err: any) { + throw new Error( + `Failed to create dbPath directory "${resolvedPath}".\n` + + ` Fix: Ensure the parent directory "${dirname(resolvedPath)}" exists and is writable,\n` + + ` or create it manually: mkdir -p "${resolvedPath}"\n` + + ` Details: ${err.code || ""} ${err.message}`, + ); + } + } + + // Check write permissions + try { + await access(resolvedPath, constants.W_OK); + } catch (err: any) { + throw new Error( + `dbPath directory "${resolvedPath}" is not writable.\n` + + ` Fix: Check permissions with: ls -la "${dirname(resolvedPath)}"\n` + + ` Or grant write access: chmod u+w "${resolvedPath}"\n` + + ` Details: ${err.code || ""} ${err.message}`, + ); + } + + return resolvedPath; +} + +async function pathExistsAsync(p: string): Promise { + try { await access(p, constants.F_OK); return true; } + catch { return false; } +} + // ============================================================================ // Memory Store // ============================================================================ From 809358d2a0c4a066e1cbfb43ffd960914465a1a2 Mon Sep 17 00:00:00 2001 From: jlin53882 Date: Mon, 11 May 2026 09:53:22 +0800 Subject: [PATCH 2/3] test(issue795): add validateStoragePathAsync unit tests Add 5 tests for validateStoragePathAsync: - resolves absolute path unchanged - creates directory if it does not exist - rejects unwritable parent directory - resolves symlink to real path - returns resolved path for existing directory Register in ci-test-manifest.mjs under core-regression group. --- scripts/ci-test-manifest.mjs | 2 + test/validate-storage-path-async.test.mjs | 87 +++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 test/validate-storage-path-async.test.mjs diff --git a/scripts/ci-test-manifest.mjs b/scripts/ci-test-manifest.mjs index 1344873b..e0c74cd9 100644 --- a/scripts/ci-test-manifest.mjs +++ b/scripts/ci-test-manifest.mjs @@ -73,6 +73,8 @@ export const CI_TEST_MANIFEST = [ { group: "core-regression", runner: "node", file: "test/command-reflection-guard.test.mjs", args: ["--test"] }, // Tier 1 memory counter fix { group: "core-regression", runner: "node", file: "test/tier1-counters.test.mjs", args: ["--test"] }, + // Issue #795 validateStoragePathAsync background init + { group: "core-regression", runner: "node", file: "test/validate-storage-path-async.test.mjs" }, ]; export function getEntriesForGroup(group) { diff --git a/test/validate-storage-path-async.test.mjs b/test/validate-storage-path-async.test.mjs new file mode 100644 index 00000000..f2ee5751 --- /dev/null +++ b/test/validate-storage-path-async.test.mjs @@ -0,0 +1,87 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync, chmodSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import jitiFactory from "jiti"; + +const jiti = jitiFactory(import.meta.url, { interopDefault: true }); +const { validateStoragePathAsync } = jiti("../src/store.ts"); + +function makeTmpDir() { + return mkdtempSync(join(tmpdir(), "memory-lancedb-pro-vsp-async-")); +} + +describe("validateStoragePathAsync", () => { + it("resolves and returns absolute path unchanged", async () => { + const dir = makeTmpDir(); + try { + const result = await validateStoragePathAsync(dir); + assert.ok(result.length > 0, "should return a non-empty string"); + assert.strictEqual(result, dir, "absolute path should be returned unchanged"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("creates the directory if it does not exist", async () => { + const dir = makeTmpDir(); + rmSync(dir, { recursive: true, force: true }); // ensure it doesn't exist + const targetPath = join(dir, "new-db"); + try { + const result = await validateStoragePathAsync(targetPath); + assert.strictEqual(result, targetPath, "should return resolved path"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("rejects path with bad permissions (unwritable parent)", async () => { + const dir = makeTmpDir(); + try { + const lockedDir = join(dir, "locked"); + const lockedFile = join(lockedDir, "subfile"); + // Create a directory that is not writable — making parent unwritable + // prevents mkdir from succeeding inside it + chmodSync(dir, 0o555); + try { + await assert.rejects( + async () => validateStoragePathAsync(join(dir, "subdir")), + (err) => err instanceof Error, + "should reject when parent directory is not writable", + ); + } finally { + chmodSync(dir, 0o755); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("resolves symlink to real path", async () => { + const dir = makeTmpDir(); + const targetDir = join(dir, "target"); + const linkPath = join(dir, "link"); + try { + // Create a dangling symlink using a file target for simplicity + writeFileSync(targetDir, "marker"); + const { symlinkSync } = await import("node:fs"); + symlinkSync(targetDir, linkPath); + const result = await validateStoragePathAsync(linkPath); + // The async version resolves symlink to real path + assert.ok(result.length > 0); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("returns resolved path for normal existing directory", async () => { + const dir = makeTmpDir(); + try { + const result = await validateStoragePathAsync(dir); + assert.strictEqual(result, dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); \ No newline at end of file From 3a612448c5e702c02858a72253f179c7de10c9db Mon Sep 17 00:00:00 2001 From: jlin53882 Date: Tue, 12 May 2026 01:10:20 +0800 Subject: [PATCH 3/3] fix(issue795): remove sync validateStoragePath from _initPluginState critical path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the blocking sync validateStoragePath() call in _initPluginState with setImmediate + validateStoragePathAsync. The 5 sync I/O operations (lstatSync/realpathSync/existsSync/mkdirSync/accessSync) are now deferred to the next event loop tick so they no longer block plugin registration. This is the '方案 B' fix — truly replacing the sync call, not just adding an async version alongside it. Sync path validation failure logs a warning instead of throwing, matching the async behavior. --- index.ts | 47 ++++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/index.ts b/index.ts index c57510c1..a87e7350 100644 --- a/index.ts +++ b/index.ts @@ -1931,36 +1931,25 @@ interface PluginSingletonState { let _singletonState: PluginSingletonState | null = null; -// Whether the async background validation (validateStoragePathAsync) has completed. -// Used to ensure background validation results are logged before hooks execute. -let _singletonAsyncValidationDone = false; - -function _startAsyncValidation(resolvedDbPath: string, api: OpenClawPluginApi): void { - // Fire-and-forget: run validateStoragePathAsync in setImmediate so the 5 sync - // I/O operations (lstat/realpath/exists/mkdir/access) do NOT block the event loop - // during plugin registration. The sync _initPluginState still runs first so - // _singletonState is always set before hooks register. This function just - // ensures the validation runs without blocking register(). - setImmediate(async () => { - try { - await validateStoragePathAsync(resolvedDbPath); - } catch (err) { - api.logger.warn( - `memory-lancedb-pro: storage path async validation failed — ${String(err)}\n` + - ` The plugin will still attempt to start, but writes may fail.`, - ); - } finally { - _singletonAsyncValidationDone = true; - } - }); -} - function _initPluginState(api: OpenClawPluginApi): PluginSingletonState { const config = parsePluginConfig(api.pluginConfig); const resolvedDbPath = api.resolvePath(config.dbPath || getDefaultDbPath()); try { - validateStoragePath(resolvedDbPath); + // Defer path validation to after _initPluginState returns but before hooks register. + // Using setImmediate moves the 5 sync I/O (lstatSync/realpathSync/existsSync/mkdirSync/accessSync) + // off the critical path. validateStoragePath failure logs a warning and allows the plugin to + // continue starting — it does not throw, so deferral is safe. + setImmediate(async () => { + try { + await validateStoragePathAsync(resolvedDbPath); + } catch (err) { + api.logger.warn( + `memory-lancedb-pro: storage path validation failed — ${String(err)}\n` + + ` The plugin will still attempt to start, but writes may fail.`, + ); + } + }); } catch (err) { api.logger.warn( `memory-lancedb-pro: storage path issue — ${String(err)}\n` + @@ -2187,14 +2176,6 @@ const memoryLanceDBProPlugin = { _registeredApis.add(api); // claim before init (Phase 2 singleton guard) _registeredApisMap.set(api, true); // dual-track: explicit claim for rollback - // Start background async validation (fire-and-forget, non-blocking). - // We do this after claiming the API slot so that if the sync _initPluginState - // below throws, the async validation will have already started but its result - // won't matter (plugin failed to start anyway). - const config = parsePluginConfig(api.pluginConfig); - const resolvedDbPath = api.resolvePath(config.dbPath || getDefaultDbPath()); - _startAsyncValidation(resolvedDbPath, api); - let singleton: typeof _singletonState; try { if (!_singletonState) { _singletonState = _initPluginState(api); }