feat(memory): allow QMD searches via mcporter keep-alive by vignesh07 · Pull Request #19617 · openclaw/openclaw

vignesh07 · 2026-02-18T01:18:07Z

Why

OpenClaw’s QMD backend currently spawns a fresh qmd process per search (QmdMemoryManager.search()), which means QMD’s local GGUF models can cold-start on every query. This adds latency and can exceed the default 4s timeout.

What

Adds an optional memory.qmd.mcporter config block that routes QMD searches through mcporter (MCP runtime) instead of spawning qmd per query.

When enabled, OpenClaw:

optionally runs mcporter daemon start (once per process) to keep stdio servers warm
calls QMD tools via: mcporter call <serverName>.<tool> --args '{...}' --output json
maps results back into MemorySearchResult[]
falls back to the existing qmd CLI path when disabled

This is designed to work with a mcporter server configured to run qmd mcp with lifecycle: keep-alive.

Config

memory:
  backend: qmd
  qmd:
    mcporter:
      enabled: true
      serverName: qmd
      startDaemon: true

Notes / followups

This PR only changes the search execution path. Indexing/update/embed still use the qmd CLI.
We should add docs/examples for the corresponding mcporter server entry.

Testing

pnpm lint
pnpm build
pnpm test (repo has existing failing tests on my run; I did not attempt to fix unrelated failures)

Greptile Summary

This PR adds an optional memory.qmd.mcporter configuration block that routes QMD memory searches through the mcporter MCP runtime instead of spawning a fresh qmd process per query, reducing cold-start latency for large GGUF models.

Key changes:

New MemoryQmdMcporterConfig type and Zod schema for the memory.qmd.mcporter config block with enabled, serverName, and startDaemon options
QmdMemoryManager.search() now branches on mcporter.enabled to route through runQmdSearchViaMcporter() instead of the existing runQmd() path
Daemon lifecycle management via ensureMcporterDaemonStarted() using globalThis singletons for once-per-process initialization
Result parsing handles mcporter's JSON envelope (structuredContent.results or direct array)
Multi-collection searches follow the same sequential best-score-by-docid pattern as the existing runQueryAcrossCollections()
Minor formatting changes (import sorting) in commands-subagents.ts and its test file
Compatibility tweak in embeddings.ts for Error cause assignment

Issues found:

Test isolation: The globalThis properties used by mcporter (__openclawMcporterDaemonStart, __openclawMcporterColdStartWarned) are not cleaned up in afterEach, which will cause test ordering dependencies if more mcporter tests are added
No retry on daemon start failure: A failed mcporter daemon start is permanently cached, so transient failures silently degrade all searches to cold-start latency for the lifetime of the process

Confidence Score: 3/5

Feature is opt-in and the core search path is unaffected when mcporter is disabled (default), but the daemon caching behavior has a design issue that could silently negate the feature's benefit.
The PR is well-structured and follows existing patterns. The mcporter path is behind a config flag (disabled by default), so existing functionality is not at risk. However, the permanently-cached failed daemon promise means a transient error on startup will silently prevent the daemon from ever starting, and the missing globalThis cleanup in tests creates fragile test isolation. These are real issues that should be addressed before or shortly after merge.
src/memory/qmd-manager.ts (daemon start caching logic), src/memory/qmd-manager.test.ts (missing globalThis cleanup in afterEach)

_{Last reviewed commit: 64ff46d}

greptile-apps

_{9 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-18T03:49:32Z

src/memory/qmd-manager.ts

+    type McporterGlobal = typeof globalThis & {
+      __openclawMcporterDaemonStart?: Promise<void>;
+    };
+    const g: McporterGlobal = globalThis;
+    if (!g.__openclawMcporterDaemonStart) {
+      g.__openclawMcporterDaemonStart = (async () => {
+        try {
+          await this.runMcporter(["daemon", "start"], { timeoutMs: 10_000 });
+        } catch (err) {
+          log.warn(`mcporter daemon start failed: ${String(err)}`);
+        }
+      })();
+    }
+    await g.__openclawMcporterDaemonStart;


Failed daemon start is permanently cached

If mcporter daemon start fails (e.g., mcporter not installed, network issue, timeout), the rejected-then-caught promise is cached on globalThis.__openclawMcporterDaemonStart forever. Every subsequent search() call will await this already-resolved promise and never retry the daemon start. The error is swallowed by the catch block on line 829, so the user only sees a single warning in the log.

For a transient failure (e.g., a timeout on first boot), the daemon will never be started for the lifetime of the process, silently degrading every search to cold-start latency — the exact problem this feature is meant to solve.

Consider either (a) not caching the promise when it fails, or (b) adding a retry mechanism with backoff.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/memory/qmd-manager.ts Line: 821-834 Comment: **Failed daemon start is permanently cached** If `mcporter daemon start` fails (e.g., `mcporter` not installed, network issue, timeout), the rejected-then-caught promise is cached on `globalThis.__openclawMcporterDaemonStart` forever. Every subsequent `search()` call will `await` this already-resolved promise and never retry the daemon start. The error is swallowed by the `catch` block on line 829, so the user only sees a single warning in the log. For a transient failure (e.g., a timeout on first boot), the daemon will never be started for the lifetime of the process, silently degrading every search to cold-start latency — the exact problem this feature is meant to solve. Consider either (a) not caching the promise when it fails, or (b) adding a retry mechanism with backoff. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-18T03:49:33Z

src/memory/embeddings.ts

+        const wrapped = new Error(combinedReason) as Error & { cause?: unknown };
+        wrapped.cause = fallbackErr;


Inconsistent Error cause pattern

This line was changed to manually assign .cause instead of using the Error constructor's cause option, but the same file still uses new Error(message, { cause: err }) on line 188 and new Error(reason, { cause: primaryErr }) on line 244. If this change was for a compatibility reason, the other call sites should be updated too; otherwise this creates an inconsistency where some errors use the constructor option and others use manual assignment.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review. Path: src/memory/embeddings.ts Line: 231-232 Comment: **Inconsistent Error `cause` pattern** This line was changed to manually assign `.cause` instead of using the Error constructor's `cause` option, but the same file still uses `new Error(message, { cause: err })` on line 188 and `new Error(reason, { cause: primaryErr })` on line 244. If this change was for a compatibility reason, the other call sites should be updated too; otherwise this creates an inconsistency where some errors use the constructor option and others use manual assignment. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-18T03:49:35Z

Additional Comments (1)

src/memory/qmd-manager.test.ts
Missing globalThis cleanup between tests

The mcporter tests at lines 924 and 965 write to globalThis.__openclawMcporterDaemonStart and globalThis.__openclawMcporterColdStartWarned, but the afterEach hook never cleans them up. Because these are process-global singletons, cached state leaks between test cases.

Concretely: the "starts the mcporter daemon only once" test (line 965) caches the daemon promise on globalThis.__openclawMcporterDaemonStart. If any future mcporter-enabled test is added after it, that test will find the stale cached promise and never spawn the daemon — causing false passes or failures depending on what it asserts. Similarly, __openclawMcporterColdStartWarned would suppress warnings in subsequent tests.

This follows the same cleanup pattern used elsewhere in the codebase (e.g., tts.test.ts saving/restoring globalThis.fetch).

  afterEach(async () => {
    vi.useRealTimers();
    delete process.env.OPENCLAW_STATE_DIR;
    delete (globalThis as Record<string, unknown>).__openclawMcporterDaemonStart;
    delete (globalThis as Record<string, unknown>).__openclawMcporterColdStartWarned;
  });

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/memory/qmd-manager.test.ts
Line: 148-151

Comment:
**Missing globalThis cleanup between tests**

The mcporter tests at lines 924 and 965 write to `globalThis.__openclawMcporterDaemonStart` and `globalThis.__openclawMcporterColdStartWarned`, but the `afterEach` hook never cleans them up. Because these are process-global singletons, cached state leaks between test cases.

Concretely: the "starts the mcporter daemon only once" test (line 965) caches the daemon promise on `globalThis.__openclawMcporterDaemonStart`. If any future mcporter-enabled test is added after it, that test will find the stale cached promise and never spawn the daemon — causing false passes or failures depending on what it asserts. Similarly, `__openclawMcporterColdStartWarned` would suppress warnings in subsequent tests.

This follows the same cleanup pattern used elsewhere in the codebase (e.g., `tts.test.ts` saving/restoring `globalThis.fetch`).

```suggestion
  afterEach(async () => {
    vi.useRealTimers();
    delete process.env.OPENCLAW_STATE_DIR;
    delete (globalThis as Record<string, unknown>).__openclawMcporterDaemonStart;
    delete (globalThis as Record<string, unknown>).__openclawMcporterColdStartWarned;
  });
```

How can I resolve this? If you propose a fix, please make it concise.

Takhoffman · 2026-02-22T00:17:53Z

Blocking (HIGH) — env split between qmd and mcporter paths

QmdMemoryManager currently uses two different env sources for subprocesses:

runQmd(...) spawns with env: this.env (agent-scoped XDG dirs)
runMcporter(...) spawns with env: process.env (global shell env)

Code refs:

src/memory/qmd-manager.ts (runQmd)
src/memory/qmd-manager.ts (runMcporter)

Why this is risky:

The manager intentionally isolates QMD state per agent via this.env (XDG_CONFIG_HOME / XDG_CACHE_HOME under the agent state dir).
mcporter-backed search bypasses that isolation and can hit a different config/index/model state than update/sync.
Result: search can return stale/empty/unexpected results even when indexing appears successful.

Required fix:

Use the same env source for mcporter subprocesses as qmd subprocesses (this.env), or explicitly pass equivalent XDG values.

Required test:

Add a targeted test in src/memory/qmd-manager.test.ts that captures spawn args and asserts mcporter invocations receive manager-scoped XDG env (same values used by qmd invocations), not raw process.env.

Takhoffman · 2026-02-22T00:55:14Z

Merged and verified.

What I fixed before landing:

High-risk env inconsistency in QdrantMemoryManager: runMcporter now uses manager-scoped this.env (same as runQmd) so mcporter and qmd resolve the same XDG state/index paths.
Added test coverage to assert mcporter spawn receives manager-scoped XDG env values.
Updated changelog text to describe the PR’s actual scope (optional memory.qmd.mcporter routing + keep-alive integration) and note the agent-scoped state handling.

Verification run on PR head:

pnpm build
pnpm check
pnpm test:macmini

Merged via squash with pinned head:

Merge commit: 3317b49d3b52c5159494a2d63b05d63aaefcacb1

@vignesh07

…nks @vignesh07 Verified: - pnpm build - pnpm check - pnpm test:macmini Co-authored-by: vignesh07 <1436853+vignesh07@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@vignesh07