fix(memory): permanent legacy sidecar migration warning for empty placeholder OpenClaw creates (#114661)

* fix(memory-core): fail closed on non-ENOENT companion stat errors

Only treat ENOENT as 'sidecar is absent'. Other fs errors (EACCES, EIO,
ELOOP, EMFILE) now return null (fail-closed) instead of silently treating
the sidecar as zero-byte, preventing legacy data from being dropped on
transient or permission failures.

* fix(memory): silence self-inflicted legacy sidecar migration warning

OpenClaw can leave a zero-byte placeholder at the legacy memory sidecar
path (~/.openclaw/memory/<agentId>.sqlite) while the live memory index
is the per-agent SQLite database. The legacy-index migration check found
the empty file, attached it, saw no legacy schema, and warned on every
CLI invocation and gateway start, with no way for users to clear it.

Treat a demonstrably empty sidecar (zero-byte main file and no WAL or
journal content) as nothing to import: remove it quietly as a migration
change instead of emitting a permanent warning. Sidecars that contain
any bytes still go through the existing import and warning paths, so a
real legacy index or a genuinely unrecognized database is unaffected.

Closes #114626

* fix(memory-core): fail closed on non-ENOENT companion stat errors

Only treat ENOENT as 'sidecar is absent'. Other fs errors (EACCES, EIO,
ELOOP, EMFILE) now return null (fail-closed) instead of silently treating
the sidecar as zero-byte, preventing legacy data from being dropped on
transient or permission failures.

* test(memory-core): add ELOOP and EACCES companion stat regression coverage

Regression tests for the fail-closed fix: when a WAL/SHM/journal
companion's fs.stat fails with ELOOP (self-referential symlink) or
EACCES (permission denied), the migration must NOT remove the main
legacy sidecar file. Only ENOENT means genuinely absent.

Addresses Whatsskill Codex review P1 finding on openclaw/openclaw#114661.

* ci: retrigger after transient runner failures
This commit is contained in:
synth
2026-08-02 10:51:20 -04:00
committed by GitHub
parent fe81123fe8
commit 6d2ff9ef38
2 changed files with 140 additions and 1 deletions
@@ -19,7 +19,7 @@ import type {
OpenKeyedStoreOptions,
PluginDoctorStateMigrationContext,
} from "openclaw/plugin-sdk/runtime-doctor";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { stateMigrations } from "./doctor-contract-api.js";
import {
DREAMING_DAILY_INGESTION_NAMESPACE,
@@ -1437,6 +1437,91 @@ describe("memory-core doctor dreaming migration", () => {
await expect(fs.access(`${legacyPath}.migrated`)).resolves.toBeUndefined();
});
it("removes an empty legacy memory sidecar placeholder without warning", async () => {
const stateDir = path.join(rootDir, "state");
const legacyPath = path.join(stateDir, "memory", "main.sqlite");
await fs.mkdir(path.dirname(legacyPath), { recursive: true });
await fs.writeFile(legacyPath, "");
const result = await legacyMemoryIndexMigration().migrateLegacyState(migrationParams());
expect(result.warnings).toEqual([]);
expect(result.changes).toEqual([
`Removed empty Memory Core legacy memory index sidecar placeholder: ${legacyPath}`,
]);
await expect(fs.access(legacyPath)).rejects.toMatchObject({ code: "ENOENT" });
});
it("preserves the main sidecar when a companion stat fails with ELOOP", async () => {
const stateDir = path.join(rootDir, "state");
const legacyPath = path.join(stateDir, "memory", "main.sqlite");
await fs.mkdir(path.dirname(legacyPath), { recursive: true });
await fs.writeFile(legacyPath, "");
// Create a self-referential symlink as the WAL companion — fs.stat will
// fail with ELOOP, which must NOT be treated as "sidecar is absent".
const walPath = `${legacyPath}-wal`;
await fs.symlink(walPath, walPath);
const result = await legacyMemoryIndexMigration().migrateLegacyState(migrationParams());
// Fail-closed: the migration must NOT remove the main file because the
// WAL companion's state is unknown (ELOOP). No removal change and no
// crash — the migration should skip the empty-sidecar cleanup path.
expect(result.changes).toEqual([]);
await expect(fs.access(legacyPath)).resolves.toBeUndefined();
});
it("preserves the main sidecar when a companion stat fails with EACCES", async () => {
const stateDir = path.join(rootDir, "state");
const legacyPath = path.join(stateDir, "memory", "main.sqlite");
await fs.mkdir(path.dirname(legacyPath), { recursive: true });
await fs.writeFile(legacyPath, "");
// Create a WAL companion then make it unreadable — fs.stat will fail
// with EACCES on some platforms, or succeed on others (running as root).
// We mock fs.stat to simulate the EACCES failure deterministically.
const walPath = `${legacyPath}-wal`;
await fs.writeFile(walPath, "");
const originalStat = fs.stat;
vi.spyOn(fs, "stat").mockImplementation(async (p: unknown, ...args: unknown[]) => {
if (typeof p === "string" && p === walPath) {
const err = new Error("EACCES: permission denied") as NodeJS.ErrnoException;
err.code = "EACCES";
throw err;
}
return originalStat.call(fs, p as string, ...(args as []));
});
try {
const result = await legacyMemoryIndexMigration().migrateLegacyState(migrationParams());
// Fail-closed: the migration must NOT remove the main file.
expect(result.changes).toEqual([]);
await expect(fs.access(legacyPath)).resolves.toBeUndefined();
} finally {
vi.restoreAllMocks();
}
});
it("keeps the schema warning for a non-empty sidecar that is not a legacy index", async () => {
const stateDir = path.join(rootDir, "state");
const legacyPath = path.join(stateDir, "memory", "main.sqlite");
await fs.mkdir(path.dirname(legacyPath), { recursive: true });
const db = new DatabaseSync(legacyPath);
try {
db.exec("CREATE TABLE unrelated (value TEXT)");
} finally {
db.close();
}
const result = await legacyMemoryIndexMigration().migrateLegacyState(migrationParams());
expect(result.changes).toEqual([]);
expect(result.warnings).toEqual([
"Skipped Memory Core legacy memory index import for agent main because the sidecar schema is not a legacy memory index",
]);
await expect(fs.access(legacyPath)).resolves.toBeUndefined();
});
it("creates migrated FTS tables with the configured legacy tokenizer", async () => {
const stateDir = path.join(rootDir, "state");
const legacyPath = path.join(stateDir, "memory", "main.sqlite");
@@ -335,6 +335,37 @@ async function preserveLegacyMemorySidecarRetryPath(params: {
);
}
/**
* List the sidecar files when every persisted byte is absent: the main file is
* zero bytes and no WAL/journal sidecar holds content. Returns null when any
* file carries bytes, because WAL frames alone can contain legacy rows.
*/
async function listEmptyLegacySidecarFiles(legacyPath: string): Promise<string[] | null> {
const emptyFiles: string[] = [];
for (const suffix of LEGACY_MEMORY_SIDECAR_SUFFIXES) {
const candidate = `${legacyPath}${suffix}`;
try {
const stat = await fs.stat(candidate);
if (!stat.isFile()) {
continue;
}
if (stat.size > 0) {
return null;
}
emptyFiles.push(candidate);
} catch (err: unknown) {
// Only ENOENT means the sidecar is genuinely absent (never written or
// cleaned up by SQLite). Other errors (EACCES, EIO, ELOOP, EMFILE)
// mean we cannot determine the state — fail closed by treating the
// sidecar as non-empty so no legacy data is silently dropped.
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
return null;
}
}
}
return emptyFiles.length > 0 ? emptyFiles : null;
}
async function migrateLegacyMemorySidecarSource(params: {
source: LegacyMemorySidecarSource;
config: unknown;
@@ -342,6 +373,29 @@ async function migrateLegacyMemorySidecarSource(params: {
changes: string[];
warnings: string[];
}): Promise<{ archiveReady: boolean }> {
// OpenClaw itself can leave a zero-byte placeholder at the legacy sidecar
// path while the live index is the per-agent SQLite database. An empty file
// holds no legacy rows, so remove it quietly instead of emitting a permanent
// self-inflicted "not a legacy memory index" warning.
const emptySidecarFiles = await listEmptyLegacySidecarFiles(params.source.legacyPath);
if (emptySidecarFiles) {
let removedAll = true;
for (const emptyPath of emptySidecarFiles) {
try {
await fs.rm(emptyPath, { force: true });
} catch {
removedAll = false;
}
}
if (removedAll) {
params.changes.push(
`Removed empty Memory Core legacy memory index sidecar placeholder: ${params.source.legacyPath}`,
);
return { archiveReady: false };
}
// Fall through to the regular import path when cleanup fails so the file
// is still diagnosed instead of silently ignored.
}
await fs.mkdir(path.dirname(params.source.agentDatabasePath), { recursive: true });
const db = openNodeSqliteDatabase(params.source.agentDatabasePath, { allowExtension: true });
try {