fix(memory): detect missing qmd binary and warn user#28700
fix(memory): detect missing qmd binary and warn user#28700Jimmy-xuzimo wants to merge 2 commits intoopenclaw:mainfrom
Conversation
The 🥱 (yawn) emoji has a negative connotation of boredom, which can be interpreted as the AI being bored with the user's request. Changing it to ⏳ (hourglass) better represents the 'operation in progress but taking longer than expected' meaning of the stallSoft state. This also aligns the implementation with the documented default in the StatusReactionEmojis type definition. Fixes openclaw#28602
When memory.backend is set to "qmd" but the qmd binary is not installed, the system now: 1. Shows a clear warning during 'openclaw doctor' with actionable fix steps 2. Logs a warning at startup when QMD fallback activates 3. Provides helpful error messages pointing to installation docs This prevents silent failures where users think QMD is working but it's actually falling back to builtin backend without notice. Fixes openclaw#25910
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0601ad2a3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const resolvedCommand = isWindows && !path.extname(command) ? `${command}.cmd` : command; | ||
|
|
||
| try { | ||
| // Try to run `qmd --version` to verify the binary works | ||
| const { stdout } = await execFileAsync(resolvedCommand, ["--version"], { |
There was a problem hiding this comment.
On Windows this probe appends .cmd (resolvedCommand) and then invokes it via execFile, but .cmd launch semantics require a shell/spawn path; this can report available: false even when QMD is correctly installed. In that case openclaw doctor and startup logs will incorrectly tell users QMD is missing and suggest fallback, which is a regression for Windows users with a valid QMD setup.
Useful? React with 👍 / 👎.
| if (!qmdCheck.available) { | ||
| log.warn(`QMD binary not available: ${qmdCheck.error}`); | ||
| log.warn("Falling back to builtin memory backend. To use QMD, install it and ensure it's on PATH."); | ||
| } else if (!statusOnly) { | ||
| const cached = QMD_MANAGER_CACHE.get(cacheKey); |
There was a problem hiding this comment.
Cache reuse is now gated behind qmdCheck.available, so any probe false-negative skips the cached-manager fast path and always creates a fresh QMD manager. When the probe is wrong (for example on Windows due to .cmd probing), repeated calls rebuild and replace cache entries instead of reusing an existing manager, causing avoidable process churn and potential resource leakage from repeatedly superseded wrappers.
Useful? React with 👍 / 👎.
Greptile Summaryadds QMD binary detection to prevent silent fallback when Critical issue found:
Minor issues:
Other notes:
Confidence Score: 2/5
Last reviewed commit: 0601ad2 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!qmdCheck.available) { | ||
| log.warn(`QMD binary not available: ${qmdCheck.error}`); | ||
| log.warn("Falling back to builtin memory backend. To use QMD, install it and ensure it's on PATH."); | ||
| } else if (!statusOnly) { | ||
| const cached = QMD_MANAGER_CACHE.get(cacheKey); | ||
| if (cached) { | ||
| return { manager: cached }; | ||
| } | ||
| } |
There was a problem hiding this comment.
control flow doesn't skip QMD creation when binary unavailable. after logging "Falling back to builtin" at line 33, code continues to line 41 and tries creating QMD manager anyway. this will fail, log another error at line 68, then finally fall back to builtin. fix: wrap the QMD manager creation (lines 41-69) inside the else block so it only runs when binary is available
| if (!qmdCheck.available) { | |
| log.warn(`QMD binary not available: ${qmdCheck.error}`); | |
| log.warn("Falling back to builtin memory backend. To use QMD, install it and ensure it's on PATH."); | |
| } else if (!statusOnly) { | |
| const cached = QMD_MANAGER_CACHE.get(cacheKey); | |
| if (cached) { | |
| return { manager: cached }; | |
| } | |
| } | |
| if (!qmdCheck.available) { | |
| log.warn(`QMD binary not available: ${qmdCheck.error}`); | |
| log.warn("Falling back to builtin memory backend. To use QMD, install it and ensure it's on PATH."); | |
| // Skip QMD creation and fall through to builtin | |
| } else { | |
| if (!statusOnly) { | |
| const cached = QMD_MANAGER_CACHE.get(cacheKey); | |
| if (cached) { | |
| return { manager: cached }; | |
| } | |
| } | |
| try { | |
| const { QmdMemoryManager } = await import("./qmd-manager.js"); | |
| const primary = await QmdMemoryManager.create({ | |
| cfg: params.cfg, | |
| agentId: params.agentId, | |
| resolved, | |
| mode: statusOnly ? "status" : "full", | |
| }); | |
| if (primary) { | |
| if (statusOnly) { | |
| return { manager: primary }; | |
| } | |
| const wrapper = new FallbackMemoryManager( | |
| { | |
| primary, | |
| fallbackFactory: async () => { | |
| const { MemoryIndexManager } = await import("./manager.js"); | |
| return await MemoryIndexManager.get(params); | |
| }, | |
| }, | |
| () => QMD_MANAGER_CACHE.delete(cacheKey), | |
| ); | |
| QMD_MANAGER_CACHE.set(cacheKey, wrapper); | |
| return { manager: wrapper }; | |
| } | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : String(err); | |
| log.warn(`qmd memory unavailable; falling back to builtin: ${message}`); | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/memory/search-manager.ts
Line: 31-39
Comment:
control flow doesn't skip QMD creation when binary unavailable. after logging "Falling back to builtin" at line 33, code continues to line 41 and tries creating QMD manager anyway. this will fail, log another error at line 68, then finally fall back to builtin. fix: wrap the QMD manager creation (lines 41-69) inside the `else` block so it only runs when binary is available
```suggestion
if (!qmdCheck.available) {
log.warn(`QMD binary not available: ${qmdCheck.error}`);
log.warn("Falling back to builtin memory backend. To use QMD, install it and ensure it's on PATH.");
// Skip QMD creation and fall through to builtin
} else {
if (!statusOnly) {
const cached = QMD_MANAGER_CACHE.get(cacheKey);
if (cached) {
return { manager: cached };
}
}
try {
const { QmdMemoryManager } = await import("./qmd-manager.js");
const primary = await QmdMemoryManager.create({
cfg: params.cfg,
agentId: params.agentId,
resolved,
mode: statusOnly ? "status" : "full",
});
if (primary) {
if (statusOnly) {
return { manager: primary };
}
const wrapper = new FallbackMemoryManager(
{
primary,
fallbackFactory: async () => {
const { MemoryIndexManager } = await import("./manager.js");
return await MemoryIndexManager.get(params);
},
},
() => QMD_MANAGER_CACHE.delete(cacheKey),
);
QMD_MANAGER_CACHE.set(cacheKey, wrapper);
return { manager: wrapper };
}
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
log.warn(`qmd memory unavailable; falling back to builtin: ${message}`);
}
}
```
How can I resolve this? If you propose a fix, please make it concise.| timeout: timeoutMs, | ||
| encoding: "utf8", | ||
| }); | ||
| const version = stdout.trim().split("\n")[0]; |
There was a problem hiding this comment.
version variable assigned but never used
| const version = stdout.trim().split("\n")[0]; | |
| const { stdout } = await execFileAsync(resolvedCommand, ["--version"], { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/memory/backend-config.ts
Line: 377
Comment:
`version` variable assigned but never used
```suggestion
const { stdout } = await execFileAsync(resolvedCommand, ["--version"], {
```
How can I resolve this? If you propose a fix, please make it concise.
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit. |
2 participants
Problem
When
memory.backendis set to"qmd"but theqmdbinary is not installed on the system, the gateway would silently fall back to the builtin memory backend without any visible warning. This caused confusion for users who thought QMD was working but were actually using the builtin backend.Issue: #25910
Solution
This PR adds explicit detection and warning when the QMD binary is missing:
1.
openclaw doctorIntegrationcheckQmdBinaryAvailable()function inbackend-config.tsthat verifies the qmd binary exists and is executablenoteMemorySearchHealth()indoctor-memory-search.tsto check for QMD binary availability when backend is set to "qmd"2. Startup Warning
getMemorySearchManager()insearch-manager.tsto check QMD binary availability before attempting to create the manager3. Cross-Platform Support
.cmdextension automaticallyTesting
Added 4 new test cases in
backend-config.test.ts:available=falsewhen binary not foundavailable=truewhen binary is available.cmdextensionAdded 2 new test cases in
doctor-memory-search.test.ts:All tests pass:
Changes
src/memory/backend-config.ts: AddedcheckQmdBinaryAvailable()functionsrc/memory/backend-config.test.ts: Added tests for new functionsrc/commands/doctor-memory-search.ts: Integrated QMD binary checksrc/commands/doctor-memory-search.test.ts: Added tests for doctor integrationsrc/memory/search-manager.ts: Added startup warning for missing QMD binaryFixes #25910