diff --git a/index.ts b/index.ts index 15d9859e..a87e7350 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, @@ -1936,7 +1936,20 @@ function _initPluginState(api: OpenClawPluginApi): PluginSingletonState { 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` + @@ -2162,6 +2175,7 @@ const memoryLanceDBProPlugin = { // ======================================================================== _registeredApis.add(api); // claim before init (Phase 2 singleton guard) _registeredApisMap.set(api, true); // dual-track: explicit claim for rollback + let singleton: typeof _singletonState; try { if (!_singletonState) { _singletonState = _initPluginState(api); } 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/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 // ============================================================================ 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