Skip to content

fix(memory-core): keep QMD JSON search one-shot#91837

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/91821-memory-json-qmd-exit
Jun 10, 2026
Merged

fix(memory-core): keep QMD JSON search one-shot#91837
vincentkoc merged 1 commit into
openclaw:mainfrom
TurboTheTurtle:fix/91821-memory-json-qmd-exit

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #91821.

openclaw memory search <query> --json could print valid JSON with the QMD backend and then stay alive because the one-shot QMD CLI manager could still schedule session-start/search sync work. withManager() closes the manager after JSON output, and close() waits for pending QMD update/embed work, so a background sync can keep the CLI process alive after the result has already been written.

This keeps the repair in the QMD lifecycle boundary: CLI-mode QMD managers still initialize without watchers or boot timers, and now also skip session/search-triggered sync scheduling during one-shot searches. JSON output remains prompt; best-effort recall tracking is not moved into the blocking JSON path.

Verification

  • node scripts/run-vitest.mjs extensions/memory-core/src/cli.test.ts extensions/memory-core/src/memory/qmd-manager.test.ts — 192 passed
  • pnpm exec oxfmt --check --threads=1 extensions/memory-core/src/cli.runtime.ts extensions/memory-core/src/cli.test.ts extensions/memory-core/src/memory/qmd-manager.ts extensions/memory-core/src/memory/qmd-manager.test.ts — clean
  • git diff --check — clean
  • git log --format='%h %an <%ae> %s' upstream/main..HEAD4137a2ab05 Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com> fix(memory-core): keep qmd json search one-shot

Real behavior proof

Behavior addressed: QMD-backed memory search --json exits after printing successful JSON even when sync settings would otherwise trigger session/search update work. The proof uses a stub qmd binary that returns valid search JSON and intentionally never exits for qmd update/qmd embed; if the CLI schedules the old background update path, the command times out.

Real environment tested: macOS source checkout /Users/andy/openclaw-91821-memory-json-exit, Node 24.15.0, rebuilt local dist/index.js, temporary OPENCLAW_CONFIG_PATH, temporary OPENCLAW_STATE_DIR, QMD backend enabled, memorySearch.sync.onSessionStart=true, memorySearch.sync.onSearch=true, fake qmd command on PATH logging every invocation.

Exact steps or command run after this patch:

node dist/index.js memory search "QMD proof token" --json

Evidence after fix:

{
  "proof": "PR91837_QMD_JSON_EXIT_STUB",
  "command": "node dist/index.js memory search \"QMD proof token\" --json",
  "config": {
    "backend": "qmd",
    "sync": {
      "onSessionStart": true,
      "onSearch": true
    },
    "qmdUpdateStubWouldHang": true
  },
  "exit": {
    "code": 0,
    "signal": null
  },
  "timedOut": false,
  "durationMs": 4020,
  "stdout": "{\n  \"results\": [\n    {\n      \"path\": \"memory/proof.md\",\n      \"startLine\": 1,\n      \"endLine\": 1,\n      \"score\": 0.91,\n      \"snippet\": \"@@ -1,1\\nQMD proof token from stub\",\n      \"source\": \"memory\"\n    }\n  ]\n}",
  "stderr": "[memory] qmd manager initialized for agent \"main\" mode=cli collections=1 durationMs=210\n[memory] failed to read qmd index stats: Error: unable to open database file",
  "qmdCalls": [
    ["collection", "list", "--json"],
    ["collection", "add", "<temp-workspace>", "--name", "workspace-main", "--mask", "**/*.md"],
    ["search", "QMD proof token", "--json", "-n", "4", "-c", "workspace-main"]
  ],
  "backgroundUpdateCallCount": 0
}

Observed result after fix: the CLI printed a valid JSON result and exited with status 0 before the 7s proof timeout. The QMD invocation log shows collection setup and search only; no qmd update or qmd embed command was spawned, so the intentionally hanging update path was not reached.

What was not tested: a live installed QMD binary and real QMD index. The proof is a faithful process-lifecycle stub for the reported exit hang: it exercises the OpenClaw CLI, config loading, QMD manager creation, QMD collection/search subprocess calls, JSON output, and process exit behavior.

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: XS triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 10, 2026
@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 10, 2026, 1:21 AM ET / 05:21 UTC.

Summary
This PR records QMD manager mode, skips session-start and dirty-search sync scheduling in CLI mode, and adds a regression test for one-shot CLI search with QMD sync triggers enabled.

PR surface: Source +8, Tests +49. Total +57 across 2 files.

Reproducibility: yes. I did not run the failing current-main command, but the source path is clear: CLI search can schedule QMD sync work and manager close waits for pending update/embed work, matching the linked report and the PR's after-fix terminal proof.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The proof uses a real OpenClaw CLI command with a qmd subprocess stub rather than a live installed QMD index; this is sufficient for the lifecycle bug, but a maintainer may still want a live-QMD smoke before landing.

Maintainer options:

  1. Decide the mitigation before merge
    Land the lifecycle-bound QMD fix after normal maintainer review and CI, keeping explicit memory index commands as the path for blocking QMD refresh work.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No repair lane is needed because there are no actionable review findings; the remaining action is normal maintainer review and CI.

Security
Cleared: The diff only changes QMD manager lifecycle branching and a colocated test, with no new dependency, secret, workflow, package, install, or code-download surface.

Review details

Best possible solution:

Land the lifecycle-bound QMD fix after normal maintainer review and CI, keeping explicit memory index commands as the path for blocking QMD refresh work.

Do we have a high-confidence way to reproduce the issue?

Yes. I did not run the failing current-main command, but the source path is clear: CLI search can schedule QMD sync work and manager close waits for pending update/embed work, matching the linked report and the PR's after-fix terminal proof.

Is this the best way to solve the issue?

Yes. Fixing the QMD manager's CLI lifecycle is more maintainable than adding process.exit() after JSON output because it prevents the background work that keeps close() alive while preserving normal explicit sync commands.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 54c400a97571.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal proof from a rebuilt OpenClaw CLI showing JSON output, exit code 0, and no update/embed subprocess calls on the hang-triggering path.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies after-fix terminal proof from a rebuilt OpenClaw CLI showing JSON output, exit code 0, and no update/embed subprocess calls on the hang-triggering path.
  • remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove merge-risk: 🚨 availability: Current PR review selected no merge-risk labels.

Label justifications:

  • P2: The PR addresses a bounded memory-core CLI bug where successful QMD JSON search can keep the process alive until upstream callers time out.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies after-fix terminal proof from a rebuilt OpenClaw CLI showing JSON output, exit code 0, and no update/embed subprocess calls on the hang-triggering path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal proof from a rebuilt OpenClaw CLI showing JSON output, exit code 0, and no update/embed subprocess calls on the hang-triggering path.
Evidence reviewed

PR surface:

Source +8, Tests +49. Total +57 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 8 0 +8
Tests 1 49 0 +49
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 57 0 +57

What I checked:

  • Repository policy applied: Read the full root AGENTS.md plus the scoped extensions/AGENTS.md; the OpenClaw PR review, proof, dependency, plugin-boundary, and changed-surface guidance applied to this memory-core review. (AGENTS.md:1, 54c400a97571)
  • Current main manager path: Current main creates QMD managers with an optional mode and already returns early from initialize() for CLI mode before watchers and timers are started, but it does not persist that mode for later search-time decisions. (extensions/memory-core/src/memory/qmd-manager.ts:318, 54c400a97571)
  • Current main search/close interaction: Current main search() calls maybeWarmSession(), maybeSyncDirtySearchState(), and waitForPendingUpdateBeforeSearch(); close() then waits for pendingUpdate and queuedForcedUpdate, which is the hang-prone lifecycle described by the linked issue. (extensions/memory-core/src/memory/qmd-manager.ts:1190, 54c400a97571)
  • Current main sync scheduling source: maybeWarmSession() schedules a session-start sync when onSessionStart is enabled, and maybeSyncDirtySearchState() can await a search sync when onSearch and dirty are true; neither function has a CLI-mode guard on current main. (extensions/memory-core/src/memory/qmd-manager.ts:1731, 54c400a97571)
  • PR runtime change: The PR adds a private mode field, assigns it during initialize(), and returns early from both search-time sync helpers when the manager mode is cli. (extensions/memory-core/src/memory/qmd-manager.ts:379, 4137a2ab0575)
  • Regression coverage: The new test enables onSessionStart and onSearch for a CLI-mode QMD manager, performs a search with a CLI session key, closes the manager, and asserts no update/embed subprocess was spawned while the search subprocess did run. (extensions/memory-core/src/memory/qmd-manager.test.ts:706, 4137a2ab0575)

Likely related people:

  • Dallin Romney: Current-main blame for initialize(), CLI-mode setup, search-time sync helpers, and the CLI manager close wrapper points to the imported memory-core implementation in commit a2dd821. (role: current implementation provenance; confidence: medium; commits: a2dd8219087a; files: extensions/memory-core/src/memory/qmd-manager.ts, extensions/memory-core/src/cli.runtime.ts)
  • Vincent Koc: Git history shows QMD sync parity hooks and the latest stable release prep touching the same memory-core QMD manager surface. (role: QMD sync feature history contributor; confidence: medium; commits: d26d7c797b65, 5181e4f7c82b; files: extensions/memory-core/src/memory/qmd-manager.ts, docs/reference/memory-config.md)
  • Peter Steinberger: Recent history includes the move of the memory engine into the memory plugin and later QMD sync/runtime refactors on the same files and adjacent host SDK paths. (role: recent memory-core refactor contributor; confidence: medium; commits: cad83db8b2f7, 30c686423f12; files: extensions/memory-core/src/memory/qmd-manager.ts, extensions/memory-core/src/memory/search-manager.ts, extensions/memory-core/src/cli.runtime.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 10, 2026
@TurboTheTurtle TurboTheTurtle force-pushed the fix/91821-memory-json-qmd-exit branch from 11a832b to 28deb04 Compare June 10, 2026 04:50
@TurboTheTurtle TurboTheTurtle changed the title fix(memory-core): settle json search cleanup fix(memory-core): keep QMD JSON search one-shot Jun 10, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 10, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@openclaw-barnacle openclaw-barnacle Bot added the proof: supplied External PR includes structured after-fix real behavior proof. label Jun 10, 2026
@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

@TurboTheTurtle TurboTheTurtle force-pushed the fix/91821-memory-json-qmd-exit branch from 28deb04 to 4137a2a Compare June 10, 2026 04:54
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 10, 2026
@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 10, 2026
@vincentkoc vincentkoc self-assigned this Jun 10, 2026
@vincentkoc vincentkoc merged commit 9408380 into openclaw:main Jun 10, 2026
189 of 200 checks passed
@vincentkoc

Copy link
Copy Markdown
Member

Landed after maintainer verification:

  • Reviewed the full QMD manager lifecycle and the CLI/full manager split.
  • Structured autoreview: clean; no accepted/actionable findings.
  • Focused proof: node scripts/run-vitest.mjs extensions/memory-core/src/memory/qmd-manager.test.ts — 124 tests passed.
  • GitHub checks were green at merge time.
  • Compared with fix(memory-core): force exit after memory search --json output #91864: this PR fixes lifecycle ownership at the manager boundary instead of forcing process.exit(), preserving normal cleanup and best-effort recall behavior.

Proof gap: no live qmd binary is installed in this checkout, so live-QMD smoke was not repeated locally. The contributor's subprocess lifecycle proof and focused regression cover the reported hang.

Landed commit: 9408380

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: memory-core Extension: memory-core P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

memory search --json prints results but does not exit with QMD backend

2 participants