mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 18:30:44 +00:00
fix(memory): unify default root memory handling (#66141)
* fix(memory): unify default root memory handling * test(memory): align legacy migration expectation * docs(changelog): tag qmd root-memory fix * docs(changelog): append qmd root-memory entry * docs(changelog): dedupe qmd root-memory entry * docs(changelog): attribute qmd root-memory fix --------- Co-authored-by: mbelinky <mbelinky@users.noreply.github.com>
This commit is contained in:
@@ -797,6 +797,8 @@ describe("QmdMemoryManager", () => {
|
||||
expect(legacyCollections.has("memory-dir-main")).toBe(true);
|
||||
expect(legacyCollections.has("memory-root")).toBe(false);
|
||||
expect(legacyCollections.has("memory-dir")).toBe(false);
|
||||
expect(legacyCollections.has("memory-alt-main")).toBe(false);
|
||||
expect(legacyCollections.has("memory-alt")).toBe(false);
|
||||
});
|
||||
|
||||
it("rebinds conflicting collection name when path+pattern slot is already occupied", async () => {
|
||||
@@ -876,6 +878,87 @@ describe("QmdMemoryManager", () => {
|
||||
expect(logWarnMock).toHaveBeenCalledWith(expect.stringContaining("rebinding"));
|
||||
});
|
||||
|
||||
it("rebinds legacy memory-alt when it still owns the root slot for MEMORY.md", async () => {
|
||||
await fs.writeFile(path.join(workspaceDir, "MEMORY.md"), "# canonical root");
|
||||
cfg = {
|
||||
...cfg,
|
||||
memory: {
|
||||
backend: "qmd",
|
||||
qmd: {
|
||||
includeDefaultMemory: true,
|
||||
update: { interval: "0s", debounceMs: 60_000, onBoot: false },
|
||||
paths: [],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
const listedCollections = new Map<
|
||||
string,
|
||||
{
|
||||
path: string;
|
||||
pattern: string;
|
||||
}
|
||||
>([["memory-alt", { path: workspaceDir, pattern: "memory.md" }]]);
|
||||
const removeCalls: string[] = [];
|
||||
|
||||
spawnMock.mockImplementation((_cmd: string, args: string[]) => {
|
||||
if (args[0] === "collection" && args[1] === "list") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(
|
||||
child,
|
||||
"stdout",
|
||||
JSON.stringify(
|
||||
[...listedCollections.entries()].map(([name, info]) => ({
|
||||
name,
|
||||
path: info.path,
|
||||
mask: info.pattern,
|
||||
})),
|
||||
),
|
||||
);
|
||||
return child;
|
||||
}
|
||||
if (args[0] === "collection" && args[1] === "remove") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
const name = args[2] ?? "";
|
||||
removeCalls.push(name);
|
||||
listedCollections.delete(name);
|
||||
queueMicrotask(() => child.closeWith(0));
|
||||
return child;
|
||||
}
|
||||
if (args[0] === "collection" && args[1] === "add") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
const pathArg = args[2] ?? "";
|
||||
const name = args[args.indexOf("--name") + 1] ?? "";
|
||||
const pattern = args[args.indexOf("--glob") + 1] ?? args[args.indexOf("--mask") + 1] ?? "";
|
||||
const hasConflict = [...listedCollections.entries()].some(([existingName, info]) => {
|
||||
if (existingName === name || info.path !== pathArg) {
|
||||
return false;
|
||||
}
|
||||
const isRootPatternPair =
|
||||
(info.pattern === "MEMORY.md" || info.pattern === "memory.md") &&
|
||||
(pattern === "MEMORY.md" || pattern === "memory.md");
|
||||
return info.pattern === pattern || isRootPatternPair;
|
||||
});
|
||||
if (hasConflict) {
|
||||
emitAndClose(child, "stderr", "A collection already exists for this path and pattern", 1);
|
||||
return child;
|
||||
}
|
||||
listedCollections.set(name, { path: pathArg, pattern });
|
||||
queueMicrotask(() => child.closeWith(0));
|
||||
return child;
|
||||
}
|
||||
return createMockChild();
|
||||
});
|
||||
|
||||
const { manager } = await createManager({ mode: "full" });
|
||||
await manager.close();
|
||||
|
||||
expect(removeCalls).toContain("memory-alt");
|
||||
expect(listedCollections.has("memory-root-main")).toBe(true);
|
||||
expect(listedCollections.has("memory-alt")).toBe(false);
|
||||
expect(logWarnMock).toHaveBeenCalledWith(expect.stringContaining("rebinding"));
|
||||
});
|
||||
|
||||
it("warns instead of silently succeeding when add conflict metadata is unavailable", async () => {
|
||||
cfg = {
|
||||
...cfg,
|
||||
@@ -912,6 +995,48 @@ describe("QmdMemoryManager", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to --mask when qmd collection add rejects --glob", async () => {
|
||||
cfg = {
|
||||
...cfg,
|
||||
memory: {
|
||||
backend: "qmd",
|
||||
qmd: {
|
||||
includeDefaultMemory: true,
|
||||
update: { interval: "0s", debounceMs: 60_000, onBoot: false },
|
||||
paths: [],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
const addFlagCalls: string[] = [];
|
||||
spawnMock.mockImplementation((_cmd: string, args: string[]) => {
|
||||
if (args[0] === "collection" && args[1] === "list") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(child, "stdout", "[]");
|
||||
return child;
|
||||
}
|
||||
if (args[0] === "collection" && args[1] === "add") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
const flag = args.includes("--glob") ? "--glob" : args.includes("--mask") ? "--mask" : "";
|
||||
addFlagCalls.push(flag);
|
||||
if (flag === "--glob") {
|
||||
emitAndClose(child, "stderr", "unknown flag: --glob", 1);
|
||||
return child;
|
||||
}
|
||||
queueMicrotask(() => child.closeWith(0));
|
||||
return child;
|
||||
}
|
||||
return createMockChild();
|
||||
});
|
||||
|
||||
const { manager } = await createManager({ mode: "full" });
|
||||
await manager.close();
|
||||
|
||||
expect(addFlagCalls).toEqual(["--glob", "--mask", "--mask"]);
|
||||
expect(logWarnMock).toHaveBeenCalledWith(
|
||||
expect.stringContaining("retrying with legacy compatibility flag"),
|
||||
);
|
||||
});
|
||||
it("migrates unscoped legacy collections from plain-text collection list output", async () => {
|
||||
cfg = {
|
||||
...cfg,
|
||||
@@ -1845,6 +1970,46 @@ describe("QmdMemoryManager", () => {
|
||||
await manager.close();
|
||||
});
|
||||
|
||||
it("does not query phantom memory-alt collections when MEMORY.md exists", async () => {
|
||||
await fs.writeFile(path.join(workspaceDir, "MEMORY.md"), "# canonical root");
|
||||
cfg = {
|
||||
...cfg,
|
||||
memory: {
|
||||
backend: "qmd",
|
||||
qmd: {
|
||||
includeDefaultMemory: true,
|
||||
update: { interval: "0s", debounceMs: 60_000, onBoot: false },
|
||||
paths: [],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
spawnMock.mockImplementation((_cmd: string, args: string[]) => {
|
||||
if (args[0] === "search") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(child, "stdout", "[]");
|
||||
return child;
|
||||
}
|
||||
return createMockChild();
|
||||
});
|
||||
|
||||
const { manager, resolved } = await createManager();
|
||||
|
||||
await manager.search("test", { sessionKey: "agent:main:slack:dm:u123" });
|
||||
const maxResults = resolved.qmd?.limits.maxResults;
|
||||
if (!maxResults) {
|
||||
throw new Error("qmd maxResults missing");
|
||||
}
|
||||
const searchCalls = spawnMock.mock.calls
|
||||
.map((call: unknown[]) => call[1] as string[])
|
||||
.filter((args: string[]) => args[0] === "search");
|
||||
expect(searchCalls).toEqual([
|
||||
["search", "test", "--json", "-n", String(maxResults), "-c", "memory-root-main"],
|
||||
["search", "test", "--json", "-n", String(maxResults), "-c", "memory-dir-main"],
|
||||
]);
|
||||
await manager.close();
|
||||
});
|
||||
|
||||
it("uses explicit external custom collection names verbatim at query time", async () => {
|
||||
const sharedMirrorDir = path.join(tmpRoot, "shared-notion-mirror");
|
||||
await fs.mkdir(sharedMirrorDir);
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import crypto from "node:crypto";
|
||||
import fsSync from "node:fs";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -507,7 +508,13 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
if (!this.pathsMatch(details.path, collection.path)) {
|
||||
continue;
|
||||
}
|
||||
if (details.pattern !== collection.pattern) {
|
||||
if (
|
||||
!this.patternsMatchForManagedCollection(
|
||||
collection.path,
|
||||
details.pattern,
|
||||
collection.pattern,
|
||||
)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
return name;
|
||||
@@ -621,7 +628,14 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
if (listedLegacy.path && !this.pathsMatch(listedLegacy.path, collection.path)) {
|
||||
return false;
|
||||
}
|
||||
if (typeof listedLegacy.pattern === "string" && listedLegacy.pattern !== collection.pattern) {
|
||||
if (
|
||||
typeof listedLegacy.pattern === "string" &&
|
||||
!this.patternsMatchForManagedCollection(
|
||||
collection.path,
|
||||
listedLegacy.pattern,
|
||||
collection.pattern,
|
||||
)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
@@ -787,10 +801,10 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
}
|
||||
|
||||
private shouldRebindCollection(collection: ManagedCollection, listed: ListedCollection): boolean {
|
||||
if (typeof listed.pattern === "string" && listed.pattern !== collection.pattern) {
|
||||
return true;
|
||||
}
|
||||
if (!listed.path) {
|
||||
if (typeof listed.pattern === "string" && listed.pattern !== collection.pattern) {
|
||||
return true;
|
||||
}
|
||||
// Older qmd versions may only return names from `collection list --json`.
|
||||
// If the pattern is also missing, do not perform destructive rebinds when
|
||||
// metadata is incomplete: remove+add can permanently drop collections if
|
||||
@@ -800,9 +814,63 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
if (!this.pathsMatch(listed.path, collection.path)) {
|
||||
return true;
|
||||
}
|
||||
if (
|
||||
typeof listed.pattern === "string" &&
|
||||
!this.patternsMatchForManagedCollection(collection.path, listed.pattern, collection.pattern)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private patternsMatchForManagedCollection(
|
||||
collectionPath: string,
|
||||
leftPattern: string,
|
||||
rightPattern: string,
|
||||
): boolean {
|
||||
if (leftPattern === rightPattern) {
|
||||
return true;
|
||||
}
|
||||
return this.isEquivalentDefaultMemoryRootPattern(collectionPath, leftPattern, rightPattern);
|
||||
}
|
||||
|
||||
private isEquivalentDefaultMemoryRootPattern(
|
||||
collectionPath: string,
|
||||
leftPattern: string,
|
||||
rightPattern: string,
|
||||
): boolean {
|
||||
if (
|
||||
!this.isDefaultMemoryRootPattern(leftPattern) ||
|
||||
!this.isDefaultMemoryRootPattern(rightPattern)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
let sawCanonical = false;
|
||||
let sawLegacyFallback = false;
|
||||
for (const entry of fsSync.readdirSync(collectionPath, { withFileTypes: true })) {
|
||||
if (entry.isSymbolicLink() || !entry.isFile()) {
|
||||
continue;
|
||||
}
|
||||
if (entry.name === "MEMORY.md") {
|
||||
sawCanonical = true;
|
||||
} else if (entry.name === "memory.md") {
|
||||
sawLegacyFallback = true;
|
||||
}
|
||||
}
|
||||
if (sawCanonical && sawLegacyFallback) {
|
||||
return false;
|
||||
}
|
||||
return sawCanonical || sawLegacyFallback;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private isDefaultMemoryRootPattern(pattern: string): boolean {
|
||||
return pattern === "MEMORY.md" || pattern === "memory.md";
|
||||
}
|
||||
|
||||
private pathsMatch(left: string, right: string): boolean {
|
||||
const normalize = (value: string): string => {
|
||||
const resolved = path.isAbsolute(value)
|
||||
|
||||
Reference in New Issue
Block a user