nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor: simplify memory recovery and test setup

This commit is contained in:
Peter Steinberger 2026-03-31 15:01:01 +01:00
parent 6936033e98
commit 4fb373466e
No known key found for this signature in database
6 changed files with 189 additions and 194 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
extensions/memory-core/src/memory/manager.read-file.test.ts
View File

@ -1,51 +1,17 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import type { OpenClawConfig } from "openclaw/plugin-sdk/memory-core-host-engine-foundation";
import { readMemoryFile } from "openclaw/plugin-sdk/memory-core-host-engine-storage";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { resetEmbeddingMocks } from "./embedding.test-mocks.js";
import type { MemoryIndexManager } from "./index.js";
import { getRequiredMemoryIndexManager } from "./test-manager-helpers.js";
function createMemorySearchCfg(options: {
workspaceDir: string;
indexPath: string;
}): OpenClawConfig {
return {
agents: {
defaults: {
workspace: options.workspaceDir,
memorySearch: {
provider: "openai",
model: "mock-embed",
store: { path: options.indexPath, vector: { enabled: false } },
cache: { enabled: false },
query: { minScore: 0, hybrid: { enabled: false } },
sync: { watch: false, onSessionStart: false, onSearch: false },
},
},
list: [{ id: "main", default: true }],
},
} as OpenClawConfig;
}
describe("MemoryIndexManager.readFile", () => {
let workspaceDir: string;
let indexPath: string;
let memoryDir: string;
let manager: MemoryIndexManager | null = null;
beforeAll(async () => {
resetEmbeddingMocks();
workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-mem-read-"));
indexPath = path.join(workspaceDir, "index.sqlite");
memoryDir = path.join(workspaceDir, "memory");
await fs.mkdir(memoryDir, { recursive: true });
manager = await getRequiredMemoryIndexManager({
cfg: createMemorySearchCfg({ workspaceDir, indexPath }),
agentId: "main",
purpose: "status",
});
});
afterEach(async () => {
@ -58,16 +24,16 @@ describe("MemoryIndexManager.readFile", () => {
});
afterAll(async () => {
if (manager) {
await manager.close();
manager = null;
}
await fs.rm(workspaceDir, { recursive: true, force: true });
});
it("returns empty text when the requested file does not exist", async () => {
const relPath = "memory/2099-01-01.md";
const result = await manager!.readFile({ relPath });
const result = await readMemoryFile({
workspaceDir,
extraPaths: [],
relPath,
});
expect(result).toEqual({ text: "", path: relPath });
});
@ -77,7 +43,13 @@ describe("MemoryIndexManager.readFile", () => {
await fs.mkdir(path.dirname(absPath), { recursive: true });
await fs.writeFile(absPath, ["line 1", "line 2", "line 3"].join("\n"), "utf-8");
const result = await manager!.readFile({ relPath, from: 2, lines: 1 });
const result = await readMemoryFile({
workspaceDir,
extraPaths: [],
relPath,
from: 2,
lines: 1,
});
expect(result).toEqual({ text: "line 2", path: relPath });
});
@ -87,7 +59,13 @@ describe("MemoryIndexManager.readFile", () => {
await fs.mkdir(path.dirname(absPath), { recursive: true });
await fs.writeFile(absPath, ["alpha", "beta"].join("\n"), "utf-8");
const result = await manager!.readFile({ relPath, from: 10, lines: 5 });
const result = await readMemoryFile({
workspaceDir,
extraPaths: [],
relPath,
from: 10,
lines: 5,
});
expect(result).toEqual({ text: "", path: relPath });
});
@ -113,7 +91,11 @@ describe("MemoryIndexManager.readFile", () => {
});
try {
const result = await manager!.readFile({ relPath });
const result = await readMemoryFile({
workspaceDir,
extraPaths: [],
relPath,
});
expect(result).toEqual({ text: "", path: relPath });
} finally {
readSpy.mockRestore();

39
extensions/memory-core/src/memory/manager.readonly-recovery.test.ts
View File

@ -4,9 +4,8 @@ import path from "node:path";
import type { DatabaseSync } from "node:sqlite";
import type { OpenClawConfig } from "openclaw/plugin-sdk/memory-core-host-engine-foundation";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { resetEmbeddingMocks } from "./embedding.test-mocks.js";
import { MemoryIndexManager } from "./manager.js";
import { getRequiredMemoryIndexManager } from "./test-manager-helpers.js";
import { openMemoryDatabaseAtPath } from "./manager-sync-ops.js";
import { runMemorySyncWithReadonlyRecovery } from "./manager.js";
type ReadonlyRecoveryHarness = {
closed: boolean;
@ -36,7 +35,6 @@ type ReadonlyRecoveryHarness = {
describe("memory manager readonly recovery", () => {
let workspaceDir = "";
let indexPath = "";
let manager: MemoryIndexManager | null = null;
function createMemoryConfig(): OpenClawConfig {
return {
@ -57,15 +55,6 @@ describe("memory manager readonly recovery", () => {
} as OpenClawConfig;
}
async function createRealManager() {
manager = await getRequiredMemoryIndexManager({
cfg: createMemoryConfig(),
agentId: "main",
purpose: "status",
});
return manager;
}
function createReadonlyRecoveryHarness() {
const reopenedClose = vi.fn();
const initialClose = vi.fn();
@ -95,7 +84,6 @@ describe("memory manager readonly recovery", () => {
ensureSchema: vi.fn(),
readMeta: vi.fn(() => undefined),
};
Object.setPrototypeOf(harness, MemoryIndexManager.prototype);
return {
harness,
initialDb,
@ -105,6 +93,16 @@ describe("memory manager readonly recovery", () => {
};
}
async function runSyncWithReadonlyRecovery(
harness: ReadonlyRecoveryHarness,
params?: { reason?: string; force?: boolean; sessionFiles?: string[] },
) {
return await runMemorySyncWithReadonlyRecovery(
harness as unknown as Parameters<typeof runMemorySyncWithReadonlyRecovery>[0],
params,
);
}
function expectReadonlyRecoveryStatus(
instance: {
readonlyRecoveryAttempts: number;
@ -131,7 +129,7 @@ describe("memory manager readonly recovery", () => {
const { harness, initialClose } = createReadonlyRecoveryHarness();
harness.runSync.mockRejectedValueOnce(params.firstError).mockResolvedValueOnce(undefined);
await MemoryIndexManager.prototype.sync.call(harness as unknown as MemoryIndexManager, {
await runSyncWithReadonlyRecovery(harness, {
reason: "test",
});
@ -142,7 +140,6 @@ describe("memory manager readonly recovery", () => {
}
beforeEach(async () => {
resetEmbeddingMocks();
workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-mem-readonly-"));
indexPath = path.join(workspaceDir, "index.sqlite");
await fs.mkdir(path.join(workspaceDir, "memory"), { recursive: true });
@ -151,10 +148,6 @@ describe("memory manager readonly recovery", () => {
afterEach(async () => {
vi.restoreAllMocks();
if (manager) {
await manager.close();
manager = null;
}
await fs.rm(workspaceDir, { recursive: true, force: true });
});
@ -177,7 +170,7 @@ describe("memory manager readonly recovery", () => {
harness.runSync.mockRejectedValueOnce(new Error("embedding timeout"));
await expect(
MemoryIndexManager.prototype.sync.call(harness as unknown as MemoryIndexManager, {
runSyncWithReadonlyRecovery(harness, {
reason: "test",
}),
).rejects.toThrow("embedding timeout");
@ -187,12 +180,12 @@ describe("memory manager readonly recovery", () => {
});
it("sets busy_timeout on memory sqlite connections", async () => {
const currentManager = await createRealManager();
const db = (currentManager as unknown as { db: DatabaseSync }).db;
const db = openMemoryDatabaseAtPath(indexPath, false);
const row = db.prepare("PRAGMA busy_timeout").get() as
| { busy_timeout?: number; timeout?: number }
| undefined;
const busyTimeout = row?.busy_timeout ?? row?.timeout;
expect(busyTimeout).toBe(5000);
db.close();
});
});

190
extensions/memory-core/src/memory/manager.ts
View File

@ -58,6 +58,121 @@ const log = createSubsystemLogger("memory");
const { indexCache: INDEX_CACHE, indexCachePending: INDEX_CACHE_PENDING } =
getMemoryIndexManagerCacheStore();
type MemoryReadonlyRecoveryState = {
closed: boolean;
db: DatabaseSync;
vectorReady: Promise<boolean> | null;
vector: {
enabled: boolean;
available: boolean | null;
extensionPath?: string;
loadError?: string;
dims?: number;
};
readonlyRecoveryAttempts: number;
readonlyRecoverySuccesses: number;
readonlyRecoveryFailures: number;
readonlyRecoveryLastError?: string;
runSync: (params?: {
reason?: string;
force?: boolean;
sessionFiles?: string[];
progress?: (update: MemorySyncProgressUpdate) => void;
}) => Promise<void>;
openDatabase: () => DatabaseSync;
ensureSchema: () => void;
readMeta: () => { vectorDims?: number } | undefined;
};
export function isMemoryReadonlyDbError(err: unknown): boolean {
const readonlyPattern =
/attempt to write a readonly database|database is read-only|SQLITE_READONLY/i;
const messages = new Set<string>();
const pushValue = (value: unknown): void => {
if (typeof value !== "string") {
return;
}
const normalized = value.trim();
if (!normalized) {
return;
}
messages.add(normalized);
};
pushValue(err instanceof Error ? err.message : String(err));
if (err && typeof err === "object") {
const record = err as Record<string, unknown>;
pushValue(record.message);
pushValue(record.code);
pushValue(record.name);
if (record.cause && typeof record.cause === "object") {
const cause = record.cause as Record<string, unknown>;
pushValue(cause.message);
pushValue(cause.code);
pushValue(cause.name);
}
}
return [...messages].some((value) => readonlyPattern.test(value));
}
export function extractMemoryErrorReason(err: unknown): string {
if (err instanceof Error && err.message.trim()) {
return err.message;
}
if (err && typeof err === "object") {
const record = err as Record<string, unknown>;
if (typeof record.message === "string" && record.message.trim()) {
return record.message;
}
if (typeof record.code === "string" && record.code.trim()) {
return record.code;
}
}
return String(err);
}
export async function runMemorySyncWithReadonlyRecovery(
state: MemoryReadonlyRecoveryState,
params?: {
reason?: string;
force?: boolean;
sessionFiles?: string[];
progress?: (update: MemorySyncProgressUpdate) => void;
},
): Promise<void> {
try {
await state.runSync(params);
return;
} catch (err) {
if (!isMemoryReadonlyDbError(err) || state.closed) {
throw err;
}
const reason = extractMemoryErrorReason(err);
state.readonlyRecoveryAttempts += 1;
state.readonlyRecoveryLastError = reason;
log.warn(`memory sync readonly handle detected; reopening sqlite connection`, { reason });
try {
state.db.close();
} catch {}
state.db = state.openDatabase();
state.vectorReady = null;
state.vector.available = null;
state.vector.loadError = undefined;
state.ensureSchema();
const meta = state.readMeta();
state.vector.dims = meta?.vectorDims;
try {
await state.runSync(params);
state.readonlyRecoverySuccesses += 1;
} catch (retryErr) {
state.readonlyRecoveryFailures += 1;
throw retryErr;
}
}
}
export async function closeAllMemoryIndexManagers(): Promise<void> {
const pending = Array.from(INDEX_CACHE_PENDING.values());
if (pending.length > 0) {
@ -598,52 +713,11 @@ export class MemoryIndexManager extends MemoryManagerEmbeddingOps implements Mem
}
private isReadonlyDbError(err: unknown): boolean {
const readonlyPattern =
/attempt to write a readonly database|database is read-only|SQLITE_READONLY/i;
const messages = new Set<string>();
const pushValue = (value: unknown): void => {
if (typeof value !== "string") {
return;
}
const normalized = value.trim();
if (!normalized) {
return;
}
messages.add(normalized);
};
pushValue(err instanceof Error ? err.message : String(err));
if (err && typeof err === "object") {
const record = err as Record<string, unknown>;
pushValue(record.message);
pushValue(record.code);
pushValue(record.name);
if (record.cause && typeof record.cause === "object") {
const cause = record.cause as Record<string, unknown>;
pushValue(cause.message);
pushValue(cause.code);
pushValue(cause.name);
}
}
return [...messages].some((value) => readonlyPattern.test(value));
return isMemoryReadonlyDbError(err);
}
private extractErrorReason(err: unknown): string {
if (err instanceof Error && err.message.trim()) {
return err.message;
}
if (err && typeof err === "object") {
const record = err as Record<string, unknown>;
if (typeof record.message === "string" && record.message.trim()) {
return record.message;
}
if (typeof record.code === "string" && record.code.trim()) {
return record.code;
}
}
return String(err);
return extractMemoryErrorReason(err);
}
private async runSyncWithReadonlyRecovery(params?: {
@ -652,35 +726,7 @@ export class MemoryIndexManager extends MemoryManagerEmbeddingOps implements Mem
sessionFiles?: string[];
progress?: (update: MemorySyncProgressUpdate) => void;
}): Promise<void> {
try {
await this.runSync(params);
return;
} catch (err) {
if (!this.isReadonlyDbError(err) || this.closed) {
throw err;
}
const reason = this.extractErrorReason(err);
this.readonlyRecoveryAttempts += 1;
this.readonlyRecoveryLastError = reason;
log.warn(`memory sync readonly handle detected; reopening sqlite connection`, { reason });
try {
this.db.close();
} catch {}
this.db = this.openDatabase();
this.vectorReady = null;
this.vector.available = null;
this.vector.loadError = undefined;
this.ensureSchema();
const meta = this.readMeta();
this.vector.dims = meta?.vectorDims;
try {
await this.runSync(params);
this.readonlyRecoverySuccesses += 1;
} catch (retryErr) {
this.readonlyRecoveryFailures += 1;
throw retryErr;
}
}
await runMemorySyncWithReadonlyRecovery(this, params);
}
async readFile(params: {

29
src/infra/path-env.test.ts
View File

@ -1,5 +1,6 @@
import path from "node:path";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { ensureOpenClawCliOnPath } from "./path-env.js";
const state = vi.hoisted(() => ({
dirs: new Set<string>(),
@ -19,15 +20,21 @@ vi.mock("node:fs", async (importOriginal) => {
...actual,
constants: { ...actual.constants, X_OK: actual.constants.X_OK ?? 1 },
accessSync: (p: string, mode?: number) => {
// `mode` is ignored in tests; we only model "is executable" or "not".
if (!state.executables.has(absInMock(p))) {
throw new Error(`EACCES: permission denied, access '${p}' (mode=${mode ?? 0})`);
const resolved = absInMock(p);
if (state.executables.has(resolved)) {
return;
}
actual.accessSync(p, mode);
},
statSync: (p: string) => {
const resolved = absInMock(p);
if (state.dirs.has(resolved)) {
return {
isDirectory: () => true,
};
}
return actual.statSync(p);
},
statSync: (p: string) => ({
// Avoid throws for non-existent paths; the code under test only cares about isDirectory().
isDirectory: () => state.dirs.has(absInMock(p)),
}),
};
return { ...wrapped, default: wrapped };
@ -37,8 +44,6 @@ vi.mock("./env.js", () => ({
isTruthyEnvValue: (value?: string) => value === "1" || value === "true",
}));
let ensureOpenClawCliOnPath: typeof import("./path-env.js").ensureOpenClawCliOnPath;
describe("ensureOpenClawCliOnPath", () => {
const envKeys = [
"PATH",
@ -51,10 +56,6 @@ describe("ensureOpenClawCliOnPath", () => {
] as const;
let envSnapshot: Record<(typeof envKeys)[number], string | undefined>;
beforeAll(async () => {
({ ensureOpenClawCliOnPath } = await import("./path-env.js"));
});
beforeEach(() => {
envSnapshot = Object.fromEntries(envKeys.map((k) => [k, process.env[k]])) as typeof envSnapshot;
state.dirs.clear();

11
src/infra/provider-usage.fetch.shared.test.ts
View File

@ -9,7 +9,6 @@ import {
describe("provider usage fetch shared helpers", () => {
afterEach(() => {
vi.useRealTimers();
vi.restoreAllMocks();
});
@ -31,7 +30,6 @@ describe("provider usage fetch shared helpers", () => {
});
it("forwards request init and clears the timeout on success", async () => {
vi.useFakeTimers();
const clearTimeoutSpy = vi.spyOn(globalThis, "clearTimeout");
const fetchFnMock = vi.fn(
async (_input: URL | RequestInfo, init?: RequestInit) =>
@ -62,7 +60,6 @@ describe("provider usage fetch shared helpers", () => {
});
it("aborts timed out requests and clears the timer on rejection", async () => {
vi.useFakeTimers();
const clearTimeoutSpy = vi.spyOn(globalThis, "clearTimeout");
const fetchFnMock = vi.fn(
(_input: URL | RequestInfo, init?: RequestInit) =>
@ -74,11 +71,9 @@ describe("provider usage fetch shared helpers", () => {
);
const fetchFn = withFetchPreconnect(fetchFnMock);
const request = fetchJson("https://example.com/usage", {}, 50, fetchFn);
const rejection = expect(request).rejects.toThrow("aborted by timeout");
await vi.advanceTimersByTimeAsync(50);
await rejection;
await expect(fetchJson("https://example.com/usage", {}, 10, fetchFn)).rejects.toThrow(
"aborted by timeout",
);
expect(clearTimeoutSpy).toHaveBeenCalledTimes(1);
});

46
src/media-understanding/image.test.ts
View File

@ -37,7 +37,8 @@ vi.mock("@mariozechner/pi-ai", async (importOriginal) => {
};
});
vi.mock("../agents/models-config.js", () => ({
vi.mock("../agents/models-config.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../agents/models-config.js")>()),
ensureOpenClawModelsJson: ensureOpenClawModelsJsonMock,
}));
@ -54,7 +55,7 @@ vi.mock("../agents/pi-model-discovery-runtime.js", () => ({
discoverModels: discoverModelsMock,
}));
let describeImageWithModel: typeof import("./image.js").describeImageWithModel;
const { describeImageWithModel } = await import("./image.js");
describe("describeImageWithModel", () => {
afterEach(() => {
@ -62,31 +63,8 @@ describe("describeImageWithModel", () => {
vi.restoreAllMocks();
});
beforeEach(async () => {
vi.resetModules();
beforeEach(() => {
vi.stubGlobal("fetch", fetchMock);
vi.doMock("@mariozechner/pi-ai", async (importOriginal) => {
const actual = await importOriginal<typeof import("@mariozechner/pi-ai")>();
return {
...actual,
complete: completeMock,
};
});
vi.doMock("../agents/models-config.js", () => ({
ensureOpenClawModelsJson: ensureOpenClawModelsJsonMock,
}));
vi.doMock("../agents/model-auth.js", () => ({
getApiKeyForModel: getApiKeyForModelMock,
resolveApiKeyForProvider: resolveApiKeyForProviderMock,
requireApiKey: requireApiKeyMock,
}));
vi.doMock("../agents/pi-model-discovery-runtime.js", () => ({
discoverAuthStorage: () => ({
setRuntimeApiKey: setRuntimeApiKeyMock,
}),
discoverModels: discoverModelsMock,
}));
({ describeImageWithModel } = await import("./image.js"));
vi.clearAllMocks();
fetchMock.mockResolvedValue({
ok: true,
@ -248,10 +226,10 @@ describe("describeImageWithModel", () => {
it("normalizes deprecated google flash ids before lookup and keeps profile auth selection", async () => {
const findMock = vi.fn((provider: string, modelId: string) => {
expect(provider).toBe("google");
expect(modelId).toBe("gemini-3-flash-preview");
expect(modelId).toBe("gemini-3.1-flash-preview");
return {
provider: "google",
id: "gemini-3-flash-preview",
id: "gemini-3.1-flash-preview",
input: ["text", "image"],
baseUrl: "https://generativelanguage.googleapis.com/v1beta",
};
@ -261,7 +239,7 @@ describe("describeImageWithModel", () => {
role: "assistant",
api: "google-generative-ai",
provider: "google",
model: "gemini-3-flash-preview",
model: "gemini-3.1-flash-preview",
stopReason: "stop",
timestamp: Date.now(),
content: [{ type: "text", text: "flash ok" }],
@ -282,7 +260,7 @@ describe("describeImageWithModel", () => {
expect(result).toEqual({
text: "flash ok",
model: "gemini-3-flash-preview",
model: "gemini-3.1-flash-preview",
});
expect(findMock).toHaveBeenCalledOnce();
expect(getApiKeyForModelMock).toHaveBeenCalledWith(
@ -296,10 +274,10 @@ describe("describeImageWithModel", () => {
it("normalizes gemini 3.1 flash-lite ids before lookup and keeps profile auth selection", async () => {
const findMock = vi.fn((provider: string, modelId: string) => {
expect(provider).toBe("google");
expect(modelId).toBe("gemini-3.1-flash-lite-preview");
expect(modelId).toBe("gemini-3.1-flash-lite");
return {
provider: "google",
id: "gemini-3.1-flash-lite-preview",
id: "gemini-3.1-flash-lite",
input: ["text", "image"],
baseUrl: "https://generativelanguage.googleapis.com/v1beta",
};
@ -309,7 +287,7 @@ describe("describeImageWithModel", () => {
role: "assistant",
api: "google-generative-ai",
provider: "google",
model: "gemini-3.1-flash-lite-preview",
model: "gemini-3.1-flash-lite",
stopReason: "stop",
timestamp: Date.now(),
content: [{ type: "text", text: "flash lite ok" }],
@ -330,7 +308,7 @@ describe("describeImageWithModel", () => {
expect(result).toEqual({
text: "flash lite ok",
model: "gemini-3.1-flash-lite-preview",
model: "gemini-3.1-flash-lite",
});
expect(findMock).toHaveBeenCalledOnce();
expect(getApiKeyForModelMock).toHaveBeenCalledWith(