fix(memory): remove sync-on-miss from memory_search tool handler (fixes #98520)#98572
Conversation
The memory_search tool performed a synchronous forced sync when the first search returned zero results (rawResults.length === 0). This blocked the foreground tool call for the full duration of the memory sync — potentially minutes when the embedding provider is slow or unreachable. The manager's search() method already handles bootstrap sync for fresh processes with no indexed content. The tool-level sync-on-miss was redundant and harmful: it re-triggered a full sync on every empty result, not just on the first search after restart. Changes: - tools.ts: Remove the rawResults.length === 0 sync-on-miss block - manager.ts: Add 15s timeout to the bootstrap sync so it does not block indefinitely when the embedding provider is unreachable - tools.test.ts: Update two tests that depended on the retry-on-miss behavior to reflect the new single-search-call semantics Fixes openclaw#98520
openclaw-barnacle
Bot
added
extensions: memory-core
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: the same QMD zero-hit memory_search stall is already owned by the open, proof-positive, maintainer-marked canonical PR at #90030, while this branch is broader and removes fallback behavior outside the reported QMD path. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this branch and continue review or landing on #90030, which fixes the QMD hot-path stall without removing the broader retry contract. So I’m closing this here and keeping the remaining discussion on #90030. Review detailsBest possible solution: Close this branch and continue review or landing on #90030, which fixes the QMD hot-path stall without removing the broader retry contract. Do we have a high-confidence way to reproduce the issue? Yes. Current main still awaits a forced sync after zero raw memory_search results, and QMD sync awaits update maintenance; the canonical issue also has repeated live reports from QMD-backed deployments. Is this the best way to solve the issue? No. The best fix is the narrower canonical QMD-only path in #90030, because this PR removes retry behavior for unaffected backends and adds separate bootstrap timeout policy. Security review: Security review cleared: No concrete security or supply-chain concern found; the diff is limited to memory-core runtime behavior and tests. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against c99add65a223. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
2 participants
What Problem This Solves
memory_searchblocks the foreground tool call for the full duration of a memory sync when the first search returns zero results. The tool handler inextensions/memory-core/src/tools.tsperforms a synchronousmanager.sync({ reason: "search", force: true })on every empty-result search, which can take minutes when the embedding provider is slow or unreachable.This makes a normal "no results" search miss behave like a full memory indexing/sync operation inside the user-facing tool call deadline.
Why This Change Was Made
The tool-level sync-on-miss (lines 569-578 in
tools.ts) is redundant with the manager's bootstrap sync inmanager.ts:631-642. The manager already handles the "fresh process with no indexed content" case with its own synchronous bootstrap. The tool-level retry adds a second full sync on every empty result — not just the first search after restart.Root cause: The tool handler called
manager.sync({ force: true })wheneverrawResults.length === 0, regardless of whether the index was already populated. For queries with no matches, this triggered a full sync cycle (filesystem scan, embedding calls, vector updates) that blocked the foreground search.Fix: Remove the tool-level sync-on-miss block. The manager's bootstrap sync handles the edge case of a fresh process with no indexed content. Additionally, add a 15-second timeout to the bootstrap sync so it does not block indefinitely when the embedding provider is unreachable.
User Impact
memory_searchfor queries with no matches returns immediately instead of blocking for minutesEvidence
Real behavior proof
upstream/main)pnpm build), rannode openclaw.mjs memory search 'nonexistent_test_query_xyz_12345' --jsonChanges Made
extensions/memory-core/src/tools.ts: Remove therawResults.length === 0 && activeMemory.manager.syncsync-on-miss block (lines 569-578)extensions/memory-core/src/memory/manager.ts: Add 15-secondPromise.racetimeout to the bootstrap sync insearch()methodextensions/memory-core/src/tools.test.ts: Update two tests that depended on the retry-on-miss behaviorHow to Test