mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(memory): harden qmd collection recovery
This commit is contained in:
@@ -118,6 +118,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Memory/Embeddings: apply configured remote-base host pinning (`allowedHostnames`) across OpenAI/Voyage/Gemini embedding requests to keep private/self-hosted endpoints working without cross-host drift. (#18198) Thanks @ianpcook.
|
||||
- Memory/Batch: route OpenAI/Voyage/Gemini batch upload/create/status/download requests through the same guarded HTTP path for consistent SSRF policy enforcement.
|
||||
- Memory/QMD: on Windows, resolve bare `qmd`/`mcporter` command names to npm shim executables (`.cmd`) before spawning, so qmd boot updates and mcporter-backed searches no longer fail with `spawn ... ENOENT` on default npm installs. (#23899) Thanks @arcbuilder-ai.
|
||||
- Memory/QMD: parse plain-text `qmd collection list --json` output when older qmd builds ignore JSON mode, and retry memory searches once after re-ensuring managed collections when qmd returns `Collection not found ...`. (#23613) Thanks @leozhucn.
|
||||
- Signal/RPC: guard malformed Signal RPC JSON responses with a clear status-scoped error and add regression coverage for invalid JSON responses. (#22995) Thanks @adhitShet.
|
||||
- Gateway/Subagents: guard gateway and subagent session-key/message trim paths against undefined inputs to prevent early `Cannot read properties of undefined (reading 'trim')` crashes during subagent spawn and wait flows.
|
||||
- Agents/Workspace: guard `resolveUserPath` against undefined/null input to prevent `Cannot read properties of undefined (reading 'trim')` crashes when workspace paths are missing in embedded runner flows.
|
||||
|
||||
@@ -496,6 +496,65 @@ describe("QmdMemoryManager", () => {
|
||||
expect(legacyCollections.has("memory-dir")).toBe(false);
|
||||
});
|
||||
|
||||
it("migrates unscoped legacy collections from plain-text collection list output", async () => {
|
||||
cfg = {
|
||||
...cfg,
|
||||
memory: {
|
||||
backend: "qmd",
|
||||
qmd: {
|
||||
includeDefaultMemory: true,
|
||||
update: { interval: "0s", debounceMs: 60_000, onBoot: false },
|
||||
paths: [],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
const removeCalls: string[] = [];
|
||||
const addCalls: string[] = [];
|
||||
spawnMock.mockImplementation((_cmd: string, args: string[]) => {
|
||||
if (args[0] === "collection" && args[1] === "list") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(
|
||||
child,
|
||||
"stdout",
|
||||
[
|
||||
"Collections (3):",
|
||||
"",
|
||||
"memory-root (qmd://memory-root/)",
|
||||
" Pattern: MEMORY.md",
|
||||
"",
|
||||
"memory-alt (qmd://memory-alt/)",
|
||||
" Pattern: memory.md",
|
||||
"",
|
||||
"memory-dir (qmd://memory-dir/)",
|
||||
" Pattern: **/*.md",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
return child;
|
||||
}
|
||||
if (args[0] === "collection" && args[1] === "remove") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
removeCalls.push(args[2] ?? "");
|
||||
queueMicrotask(() => child.closeWith(0));
|
||||
return child;
|
||||
}
|
||||
if (args[0] === "collection" && args[1] === "add") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
addCalls.push(args[args.indexOf("--name") + 1] ?? "");
|
||||
queueMicrotask(() => child.closeWith(0));
|
||||
return child;
|
||||
}
|
||||
return createMockChild();
|
||||
});
|
||||
|
||||
const { manager } = await createManager({ mode: "full" });
|
||||
await manager.close();
|
||||
|
||||
expect(removeCalls).toEqual(["memory-root", "memory-alt", "memory-dir"]);
|
||||
expect(addCalls).toEqual(["memory-root-main", "memory-alt-main", "memory-dir-main"]);
|
||||
});
|
||||
|
||||
it("does not migrate unscoped collections when listed metadata differs", async () => {
|
||||
cfg = {
|
||||
...cfg,
|
||||
@@ -729,6 +788,96 @@ describe("QmdMemoryManager", () => {
|
||||
await manager.close();
|
||||
});
|
||||
|
||||
it("repairs missing managed collections and retries search once", async () => {
|
||||
cfg = {
|
||||
...cfg,
|
||||
memory: {
|
||||
backend: "qmd",
|
||||
qmd: {
|
||||
includeDefaultMemory: true,
|
||||
searchMode: "search",
|
||||
update: { interval: "0s", debounceMs: 60_000, onBoot: false },
|
||||
paths: [],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
const expectedDocId = "abc123";
|
||||
let missingCollectionSeen = false;
|
||||
let addCallsAfterMissing = 0;
|
||||
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") {
|
||||
if (missingCollectionSeen) {
|
||||
addCallsAfterMissing += 1;
|
||||
}
|
||||
return createMockChild();
|
||||
}
|
||||
if (args[0] === "search") {
|
||||
const collectionFlagIndex = args.indexOf("-c");
|
||||
const collection = collectionFlagIndex >= 0 ? args[collectionFlagIndex + 1] : "";
|
||||
if (collection === "memory-root-main" && !missingCollectionSeen) {
|
||||
missingCollectionSeen = true;
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(child, "stderr", "Collection not found: memory-root-main", 1);
|
||||
return child;
|
||||
}
|
||||
if (collection === "memory-root-main") {
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(
|
||||
child,
|
||||
"stdout",
|
||||
JSON.stringify([{ docid: expectedDocId, score: 1, snippet: "@@ -1,1\nremember this" }]),
|
||||
);
|
||||
return child;
|
||||
}
|
||||
const child = createMockChild({ autoClose: false });
|
||||
emitAndClose(child, "stdout", "[]");
|
||||
return child;
|
||||
}
|
||||
return createMockChild();
|
||||
});
|
||||
|
||||
const { manager } = await createManager({ mode: "full" });
|
||||
const inner = manager as unknown as {
|
||||
db: { prepare: (query: string) => { all: (arg: unknown) => unknown }; close: () => void };
|
||||
};
|
||||
inner.db = {
|
||||
prepare: (_query: string) => ({
|
||||
all: (arg: unknown) => {
|
||||
if (typeof arg === "string" && arg.startsWith(expectedDocId)) {
|
||||
return [{ collection: "memory-root-main", path: "MEMORY.md" }];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
}),
|
||||
close: () => {},
|
||||
};
|
||||
|
||||
await expect(
|
||||
manager.search("remember", { sessionKey: "agent:main:slack:dm:u123" }),
|
||||
).resolves.toEqual([
|
||||
{
|
||||
path: "MEMORY.md",
|
||||
startLine: 1,
|
||||
endLine: 1,
|
||||
score: 1,
|
||||
snippet: "@@ -1,1\nremember this",
|
||||
source: "memory",
|
||||
},
|
||||
]);
|
||||
expect(addCallsAfterMissing).toBeGreaterThan(0);
|
||||
expect(logWarnMock).toHaveBeenCalledWith(
|
||||
expect.stringContaining("repairing collections and retrying once"),
|
||||
);
|
||||
|
||||
await manager.close();
|
||||
});
|
||||
|
||||
it("uses qmd.cmd on Windows when qmd command is bare", async () => {
|
||||
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
try {
|
||||
|
||||
@@ -304,29 +304,9 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
const result = await this.runQmd(["collection", "list", "--json"], {
|
||||
timeoutMs: this.qmd.update.commandTimeoutMs,
|
||||
});
|
||||
const parsed = JSON.parse(result.stdout) as unknown;
|
||||
if (Array.isArray(parsed)) {
|
||||
for (const entry of parsed) {
|
||||
if (typeof entry === "string") {
|
||||
existing.set(entry, {});
|
||||
} else if (entry && typeof entry === "object") {
|
||||
const name = (entry as { name?: unknown }).name;
|
||||
if (typeof name === "string") {
|
||||
const listedPath = (entry as { path?: unknown }).path;
|
||||
const listedPattern = (entry as { pattern?: unknown; mask?: unknown }).pattern;
|
||||
const listedMask = (entry as { mask?: unknown }).mask;
|
||||
existing.set(name, {
|
||||
path: typeof listedPath === "string" ? listedPath : undefined,
|
||||
pattern:
|
||||
typeof listedPattern === "string"
|
||||
? listedPattern
|
||||
: typeof listedMask === "string"
|
||||
? listedMask
|
||||
: undefined,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
const parsed = this.parseListedCollections(result.stdout);
|
||||
for (const [name, details] of parsed) {
|
||||
existing.set(name, details);
|
||||
}
|
||||
} catch {
|
||||
// ignore; older qmd versions might not support list --json.
|
||||
@@ -444,6 +424,22 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
);
|
||||
}
|
||||
|
||||
private isMissingCollectionSearchError(err: unknown): boolean {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
return this.isCollectionMissingError(message) && message.toLowerCase().includes("collection");
|
||||
}
|
||||
|
||||
private async tryRepairMissingCollectionSearch(err: unknown): Promise<boolean> {
|
||||
if (!this.isMissingCollectionSearchError(err)) {
|
||||
return false;
|
||||
}
|
||||
log.warn(
|
||||
"qmd search failed because a managed collection is missing; repairing collections and retrying once",
|
||||
);
|
||||
await this.ensureCollections();
|
||||
return true;
|
||||
}
|
||||
|
||||
private async addCollection(pathArg: string, name: string, pattern: string): Promise<void> {
|
||||
await this.runQmd(["collection", "add", pathArg, "--name", name, "--mask", pattern], {
|
||||
timeoutMs: this.qmd.update.commandTimeoutMs,
|
||||
@@ -456,6 +452,92 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
});
|
||||
}
|
||||
|
||||
private parseListedCollections(output: string): Map<string, ListedCollection> {
|
||||
const listed = new Map<string, ListedCollection>();
|
||||
const trimmed = output.trim();
|
||||
if (!trimmed) {
|
||||
return listed;
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(trimmed) as unknown;
|
||||
if (Array.isArray(parsed)) {
|
||||
for (const entry of parsed) {
|
||||
if (typeof entry === "string") {
|
||||
listed.set(entry, {});
|
||||
continue;
|
||||
}
|
||||
if (!entry || typeof entry !== "object") {
|
||||
continue;
|
||||
}
|
||||
const name = (entry as { name?: unknown }).name;
|
||||
if (typeof name !== "string") {
|
||||
continue;
|
||||
}
|
||||
const listedPath = (entry as { path?: unknown }).path;
|
||||
const listedPattern = (entry as { pattern?: unknown; mask?: unknown }).pattern;
|
||||
const listedMask = (entry as { mask?: unknown }).mask;
|
||||
listed.set(name, {
|
||||
path: typeof listedPath === "string" ? listedPath : undefined,
|
||||
pattern:
|
||||
typeof listedPattern === "string"
|
||||
? listedPattern
|
||||
: typeof listedMask === "string"
|
||||
? listedMask
|
||||
: undefined,
|
||||
});
|
||||
}
|
||||
return listed;
|
||||
}
|
||||
} catch {
|
||||
// Some qmd builds ignore `--json` and still print table output.
|
||||
}
|
||||
|
||||
let currentName: string | null = null;
|
||||
for (const rawLine of output.split(/\r?\n/)) {
|
||||
const line = rawLine.trimEnd();
|
||||
if (!line.trim()) {
|
||||
currentName = null;
|
||||
continue;
|
||||
}
|
||||
const collectionLine = /^\s*([a-z0-9._-]+)\s+\(qmd:\/\/[^)]+\)\s*$/i.exec(line);
|
||||
if (collectionLine) {
|
||||
currentName = collectionLine[1];
|
||||
if (!listed.has(currentName)) {
|
||||
listed.set(currentName, {});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (/^\s*collections\b/i.test(line)) {
|
||||
continue;
|
||||
}
|
||||
const bareNameLine = /^\s*([a-z0-9._-]+)\s*$/i.exec(line);
|
||||
if (bareNameLine && !line.includes(":")) {
|
||||
currentName = bareNameLine[1];
|
||||
if (!listed.has(currentName)) {
|
||||
listed.set(currentName, {});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (!currentName) {
|
||||
continue;
|
||||
}
|
||||
const patternLine = /^\s*(?:pattern|mask)\s*:\s*(.+?)\s*$/i.exec(line);
|
||||
if (patternLine) {
|
||||
const existing = listed.get(currentName) ?? {};
|
||||
existing.pattern = patternLine[1].trim();
|
||||
listed.set(currentName, existing);
|
||||
continue;
|
||||
}
|
||||
const pathLine = /^\s*path\s*:\s*(.+?)\s*$/i.exec(line);
|
||||
if (pathLine) {
|
||||
const existing = listed.get(currentName) ?? {};
|
||||
existing.path = pathLine[1].trim();
|
||||
listed.set(currentName, existing);
|
||||
}
|
||||
}
|
||||
return listed;
|
||||
}
|
||||
|
||||
private shouldRebindCollection(collection: ManagedCollection, listed: ListedCollection): boolean {
|
||||
if (!listed.path) {
|
||||
// Older qmd versions may only return names from `collection list --json`.
|
||||
@@ -547,26 +629,28 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
}
|
||||
const qmdSearchCommand = this.qmd.searchMode;
|
||||
const mcporterEnabled = this.qmd.mcporter.enabled;
|
||||
let parsed: QmdQueryResult[];
|
||||
try {
|
||||
if (mcporterEnabled) {
|
||||
const tool: "search" | "vector_search" | "deep_search" =
|
||||
qmdSearchCommand === "search"
|
||||
? "search"
|
||||
: qmdSearchCommand === "vsearch"
|
||||
? "vector_search"
|
||||
: "deep_search";
|
||||
const minScore = opts?.minScore ?? 0;
|
||||
if (collectionNames.length > 1) {
|
||||
parsed = await this.runMcporterAcrossCollections({
|
||||
tool,
|
||||
query: trimmed,
|
||||
limit,
|
||||
minScore,
|
||||
collectionNames,
|
||||
});
|
||||
} else {
|
||||
parsed = await this.runQmdSearchViaMcporter({
|
||||
const runSearchAttempt = async (
|
||||
allowMissingCollectionRepair: boolean,
|
||||
): Promise<QmdQueryResult[]> => {
|
||||
try {
|
||||
if (mcporterEnabled) {
|
||||
const tool: "search" | "vector_search" | "deep_search" =
|
||||
qmdSearchCommand === "search"
|
||||
? "search"
|
||||
: qmdSearchCommand === "vsearch"
|
||||
? "vector_search"
|
||||
: "deep_search";
|
||||
const minScore = opts?.minScore ?? 0;
|
||||
if (collectionNames.length > 1) {
|
||||
return await this.runMcporterAcrossCollections({
|
||||
tool,
|
||||
query: trimmed,
|
||||
limit,
|
||||
minScore,
|
||||
collectionNames,
|
||||
});
|
||||
}
|
||||
return await this.runQmdSearchViaMcporter({
|
||||
mcporter: this.qmd.mcporter,
|
||||
tool,
|
||||
query: trimmed,
|
||||
@@ -576,50 +660,61 @@ export class QmdMemoryManager implements MemorySearchManager {
|
||||
timeoutMs: this.qmd.limits.timeoutMs,
|
||||
});
|
||||
}
|
||||
} else if (collectionNames.length > 1) {
|
||||
parsed = await this.runQueryAcrossCollections(
|
||||
trimmed,
|
||||
limit,
|
||||
collectionNames,
|
||||
qmdSearchCommand,
|
||||
);
|
||||
} else {
|
||||
if (collectionNames.length > 1) {
|
||||
return await this.runQueryAcrossCollections(
|
||||
trimmed,
|
||||
limit,
|
||||
collectionNames,
|
||||
qmdSearchCommand,
|
||||
);
|
||||
}
|
||||
const args = this.buildSearchArgs(qmdSearchCommand, trimmed, limit);
|
||||
args.push(...this.buildCollectionFilterArgs(collectionNames));
|
||||
// Always scope to managed collections (default + custom). Even for `search`/`vsearch`,
|
||||
// pass collection filters; if a given QMD build rejects these flags, we fall back to `query`.
|
||||
const result = await this.runQmd(args, { timeoutMs: this.qmd.limits.timeoutMs });
|
||||
parsed = parseQmdQueryJson(result.stdout, result.stderr);
|
||||
}
|
||||
} catch (err) {
|
||||
if (
|
||||
!mcporterEnabled &&
|
||||
qmdSearchCommand !== "query" &&
|
||||
this.isUnsupportedQmdOptionError(err)
|
||||
) {
|
||||
log.warn(
|
||||
`qmd ${qmdSearchCommand} does not support configured flags; retrying search with qmd query`,
|
||||
);
|
||||
try {
|
||||
if (collectionNames.length > 1) {
|
||||
parsed = await this.runQueryAcrossCollections(trimmed, limit, collectionNames, "query");
|
||||
} else {
|
||||
return parseQmdQueryJson(result.stdout, result.stderr);
|
||||
} catch (err) {
|
||||
if (allowMissingCollectionRepair && this.isMissingCollectionSearchError(err)) {
|
||||
throw err;
|
||||
}
|
||||
if (
|
||||
!mcporterEnabled &&
|
||||
qmdSearchCommand !== "query" &&
|
||||
this.isUnsupportedQmdOptionError(err)
|
||||
) {
|
||||
log.warn(
|
||||
`qmd ${qmdSearchCommand} does not support configured flags; retrying search with qmd query`,
|
||||
);
|
||||
try {
|
||||
if (collectionNames.length > 1) {
|
||||
return await this.runQueryAcrossCollections(trimmed, limit, collectionNames, "query");
|
||||
}
|
||||
const fallbackArgs = this.buildSearchArgs("query", trimmed, limit);
|
||||
fallbackArgs.push(...this.buildCollectionFilterArgs(collectionNames));
|
||||
const fallback = await this.runQmd(fallbackArgs, {
|
||||
timeoutMs: this.qmd.limits.timeoutMs,
|
||||
});
|
||||
parsed = parseQmdQueryJson(fallback.stdout, fallback.stderr);
|
||||
return parseQmdQueryJson(fallback.stdout, fallback.stderr);
|
||||
} catch (fallbackErr) {
|
||||
log.warn(`qmd query fallback failed: ${String(fallbackErr)}`);
|
||||
throw fallbackErr instanceof Error ? fallbackErr : new Error(String(fallbackErr));
|
||||
}
|
||||
} catch (fallbackErr) {
|
||||
log.warn(`qmd query fallback failed: ${String(fallbackErr)}`);
|
||||
throw fallbackErr instanceof Error ? fallbackErr : new Error(String(fallbackErr));
|
||||
}
|
||||
} else {
|
||||
const label = mcporterEnabled ? "mcporter/qmd" : `qmd ${qmdSearchCommand}`;
|
||||
log.warn(`${label} failed: ${String(err)}`);
|
||||
throw err instanceof Error ? err : new Error(String(err));
|
||||
}
|
||||
};
|
||||
|
||||
let parsed: QmdQueryResult[];
|
||||
try {
|
||||
parsed = await runSearchAttempt(true);
|
||||
} catch (err) {
|
||||
if (!(await this.tryRepairMissingCollectionSearch(err))) {
|
||||
throw err instanceof Error ? err : new Error(String(err));
|
||||
}
|
||||
parsed = await runSearchAttempt(false);
|
||||
}
|
||||
const results: MemorySearchResult[] = [];
|
||||
for (const entry of parsed) {
|
||||
|
||||
Reference in New Issue
Block a user