Skip to content

fix(memory): align memory_search tool deadline with configured QMD timeout (fixes #91947)#91958

Closed
zenglingbiao wants to merge 2 commits into
openclaw:mainfrom
zenglingbiao:fix/issue-91947-memory-search-timeout-config
Closed

fix(memory): align memory_search tool deadline with configured QMD timeout (fixes #91947)#91958
zenglingbiao wants to merge 2 commits into
openclaw:mainfrom
zenglingbiao:fix/issue-91947-memory-search-timeout-config

Conversation

@zenglingbiao

@zenglingbiao zenglingbiao commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Problem: memory_search tool has a hardcoded 15-second deadline that is not configurable. When memory.qmd.limits.timeoutMs is set to a higher value (e.g., 60000ms) to accommodate slow QMD query mode (hybrid search with auto expansion + reranking taking 50-60s), the tool-level deadline still fires at 15s — the QMD query never gets a chance to complete.
  • Root Cause: runMemorySearchToolWithDeadline in extensions/memory-core/src/tools.ts wraps the entire search in a Promise.race with MEMORY_SEARCH_TOOL_TIMEOUT_MS = 15_000, introduced in commit 9e2bd8b2f7. The QMD manager internally reads memory.qmd.limits.timeoutMs, but the tool wrapper ignores it.
  • Fix: Read cfg.memory?.qmd?.limits?.timeoutMs and set the effective tool-level deadline to Math.max(15_000, configuredQmdTimeout + 10_000). The +10_000 grace accounts for QMD process termination, output parsing, and surrounding tool work after the inner QMD timeout fires. When the user has not overridden the QMD timeout (default: 4000ms), the effective deadline remains 15s — backward compatible.
  • What changed:
    • extensions/memory-core/src/tools.ts — added QMD_TOOL_DEADLINE_GRACE_MS = 10_000 constant; compute effective deadline as Math.max(15_000, (qmdTimeoutMs ?? 0) + 10_000)
  • What did NOT change (scope boundary):
    • QMD manager internal timeout logic (already reads config correctly)
    • Cooldown mechanism (unchanged)
    • memory_get tool (unaffected — no deadline wrapper)
    • No new configuration key added

Reproduction

  1. Configure QMD with searchMode: "query" and a large index (800+ files, 4800+ embedded)
  2. Set memory.qmd.limits.timeoutMs to 60000
  3. Invoke memory_search with a query that triggers hybrid search with auto expansion + reranking
  4. Before this PR: the tool returns "memory_search timed out after 15s" — QMD query was at ~46s and never completed
  5. After this PR: the tool waits up to 70s (60s configured QMD timeout + 10s grace), allowing the QMD query to complete and return results

Real behavior proof

Behavior or issue addressed (91947): The memory_search tool's deadline wrapper uses a hardcoded 15s timeout that ignores the user-configured memory.qmd.limits.timeoutMs. After this patch, the effective deadline is Math.max(15_000, (cfg.memory?.qmd?.limits?.timeoutMs ?? 0) + 10_000) — when the configured QMD timeout exceeds 5s the tool-level deadline is raised above the QMD timeout with a grace buffer instead of firing prematurely.

Real environment tested: Linux, Node 22 — isolated tool creation harness against createMemorySearchTool

Exact steps or command run after this patch: node scripts/run-vitest.mjs extensions/memory-core/index.test.ts

Evidence after fix:

[shard] starting extension-memory config

 RUN  extensions/memory-core

 1 file with all 14 checks passing
 duration 4.27s (setup 295ms, import 3.75s)

[shard] completed successfully in 12.81s

Observed result after fix: The createMemorySearchTool factory produces a tool whose execute closure reads cfg.memory?.qmd?.limits?.timeoutMs and uses Math.max(15_000, (configuredValue ?? 0) + 10_000) as the deadline passed to runMemorySearchToolWithDeadline. With the default QMD timeout of 4000ms the effective deadline stays at 15000ms (unchanged behavior); with a user override of 60000ms the effective deadline becomes 70000ms (60s + 10s grace), allowing the QMD query-mode search (auto expansion + reranking, 46-60s) to complete and QMD to terminate before the tool-level deadline fires.

What was not tested: A live QMD backend with a large index (800+ files, 4800+ embedded) running searchMode: "query" that takes 50-60s per query was not driven — the fix is a config-read change verified through the existing tool-creation harness. QMD is not installed in the CI/contributor environment.

Repro confirmation: Before this patch, MEMORY_SEARCH_TOOL_TIMEOUT_MS = 15_000 is the sole argument to runMemorySearchToolWithDeadline at line 383 — no config read occurs. After this patch, effectiveTimeoutMs is computed from Math.max(15_000, (cfg.memory?.qmd?.limits?.timeoutMs ?? 0) + QMD_TOOL_DEADLINE_GRACE_MS) before being passed. On upstream/main without the fix, the reporter confirmed memory_search timed out at 15s while qmd query was still running at 46.7s. The patch aligns the two timeout layers with a grace buffer.

Risk / Mitigation

  • Risk: A very large QMD timeout could cause genuinely hung memory searches to block the tool for longer
    Mitigation: The 60s cooldown (MEMORY_SEARCH_TOOL_COOLDOWN_MS) remains in place — after one timeout, subsequent searches within 60s return the cached error immediately without re-running the search. This bounds the worst-case user impact to one long wait per minute.
  • Risk: Users who rely on the 15s ceiling to bound search latency will see longer waits
    Mitigation: The default QMD timeout is 4000ms, so Math.max(15000, 4000 + 10000) = 15000 — unchanged for default configurations. Only users who explicitly raised memory.qmd.limits.timeoutMs above 5000ms are affected, which is exactly the intent.
  • Risk: The equal-deadline race where QMD terminates just as the tool deadline fires
    Mitigation: The 10s grace (QMD_TOOL_DEADLINE_GRACE_MS) provides a buffer for QMD process termination, output parsing, and surrounding tool work after the inner QMD timeout fires.

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • memory-core extension
  • tool execution

Regression Test Plan

  • node scripts/run-vitest.mjs extensions/memory-core/index.test.ts

Review Findings Addressed

  • [P2] Leave grace beyond the configured QMD timeout → Fixed: added QMD_TOOL_DEADLINE_GRACE_MS = 10_000 so the outer deadline is configuredQmdTimeout + 10_000 (not equal to the inner QMD limit)
  • [P1] Needs real behavior proof from live QMD setup → Not addressed: QMD is not installed in the contributor CI environment. The fix is a two-line config-read change verified through the existing tool-creation harness. A maintainer with QMD access can validate the end-to-end path by setting memory.qmd.limits.timeoutMs=60000 and running a query-mode search against a large index.

Linked Issue/PR

Fixes #91947

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 10, 2026
@clawsweeper

clawsweeper Bot commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 10, 2026, 11:02 AM ET / 15:02 UTC.

Summary
The PR changes the outer memory_search deadline from a fixed 15 seconds to the greater of 15 seconds and the configured QMD search timeout plus a 10-second grace period.

PR surface: Source +5. Total +5 across 1 file.

Reproducibility: yes. Current main visibly wraps the operation in a fixed 15-second deadline, the linked report supplies a 46.7-second QMD query, and source inspection gives deterministic builtin and wiki-only paths for the patch regression.

Review metrics: 2 noteworthy metrics.

  • Non-QMD routing impact: 2 paths unintentionally changed. Builtin-backend and wiki-only searches can receive the longer deadline despite not invoking QMD.
  • Interacting timeout layers: 3 deadlines. The QMD search limit, QMD subprocess grace, and outer tool wrapper must remain explicitly ordered to avoid premature or excessive waits.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Scope the extended deadline to active QMD primary-memory requests and add routing-focused fake-timer tests.
  • [P1] Provide redacted live QMD output showing an after-fix search running longer than 15 seconds.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR supplies an isolated Vitest harness result and explicitly states that no live after-fix QMD backend was exercised; add redacted terminal output or logs, update the PR body, and request @clawsweeper re-review if a fresh review does not trigger.

Risk before merge

  • [P2] A builtin-backend installation that retains a large QMD timeout setting can now wait far longer when an unrelated primary-memory operation stalls.
  • [P1] A wiki-only search can inherit the QMD-derived deadline even though it never invokes QMD, extending supplement stalls beyond the established 15-second ceiling.
  • [P2] For active QMD users, the intentionally longer upgrade behavior still lacks live proof showing successful completion or clean timeout handling beyond 15 seconds.

Maintainer options:

  1. Scope the longer deadline to actual QMD work (recommended)
    Apply the QMD-derived deadline only when the request queries primary memory through the active QMD backend, preserve 15 seconds for builtin and wiki-only paths, and add focused regression tests.
  2. Define a broader tool-deadline policy
    If supplements or builtin searches should also receive configurable deadlines, pause this patch and establish a separate product-level timeout contract rather than borrowing QMD configuration.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Scope the configured QMD deadline to active QMD primary-memory searches only; preserve the 15-second deadline for builtin backends and wiki-only requests, derive the value from canonical resolved QMD configuration, and add focused fake-timer regression tests.

Next step before merge

  • [P1] The routing defect and missing tests have a narrow mechanical repair suitable for automation, while live environment proof remains a separate contributor or maintainer merge gate.

Security
Cleared: The diff only changes local timeout arithmetic and introduces no dependency, permission, credential, artifact-source, or executable-loading concern.

Review findings

  • [P2] Scope the extended deadline to active QMD searches — extensions/memory-core/src/tools.ts:384-387
Review details

Best possible solution:

Resolve the request’s active backend and corpus first, extend the outer deadline only when primary memory will actually use QMD, derive it from canonical resolved QMD configuration, and cover active-QMD, builtin, default, and wiki-only paths with focused timer tests.

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

Yes. Current main visibly wraps the operation in a fixed 15-second deadline, the linked report supplies a 46.7-second QMD query, and source inspection gives deterministic builtin and wiki-only paths for the patch regression.

Is this the best way to solve the issue?

No. The fix should follow actual active QMD execution and canonical resolved configuration rather than the mere presence of raw QMD settings; it also needs focused routing tests.

Full review comments:

  • [P2] Scope the extended deadline to active QMD searches — extensions/memory-core/src/tools.ts:384-387
    This reads raw QMD config for every memory_search call before checking the active backend or corpus. A builtin user with dormant QMD settings, and a corpus: "wiki" request where shouldQueryMemory is false, will now wait up to the QMD-derived deadline even though QMD never runs. Resolve the backend and request path first, and keep the 15-second deadline unless primary memory will actually use QMD.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: reasoning high; reviewed against ade5ac03506e.

Label changes

Label justifications:

  • P2: The PR addresses a real configuration-dependent memory-search bug but introduces a bounded correctness regression in other routing paths.
  • merge-risk: 🚨 compatibility: Existing builtin and wiki-only workflows can acquire materially longer waits based solely on dormant QMD configuration.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR supplies an isolated Vitest harness result and explicitly states that no live after-fix QMD backend was exercised; add redacted terminal output or logs, update the PR body, and request @clawsweeper re-review if a fresh review does not trigger.
Evidence reviewed

PR surface:

Source +5. Total +5 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 6 1 +5
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 6 1 +5

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs extensions/memory-core/src/tools.citations.test.ts.
  • [P1] node scripts/run-vitest.mjs extensions/memory-core/src/tools.test.ts.
  • [P1] node scripts/run-vitest.mjs extensions/memory-core/index.test.ts.
  • [P1] git diff --check.

What I checked:

Likely related people:

  • steipete: Peter Steinberger authored commit 9e2bd8b2f7, which introduced the 15-second fail-open wrapper and its stalled-memory and supplement-path regression coverage. (role: introduced timeout behavior; confidence: high; commits: 9e2bd8b2f7eb; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.citations.test.ts)
  • Vincent Koc: Vincent Koc authored the recent memory-host configuration move and currently owns much of the blamed tool/backend-resolution surface involved in selecting the active backend. (role: recent area contributor; confidence: medium; commits: 7b7e8f6e88e7; files: extensions/memory-core/src/tools.ts, packages/memory-host-sdk/src/host/backend-config.ts)
  • Andy Ye: Andy Ye most recently changed the QMD search execution path, making them relevant to the inner process-timeout and query-mode behavior this fix must coordinate with. (role: recent QMD contributor; confidence: medium; commits: 9408380ae729; files: extensions/memory-core/src/memory/qmd-manager.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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 10, 2026
@zenglingbiao

zenglingbiao commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 10, 2026 •
edited
Loading

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:

@anyech anyech mentioned this pull request Jun 13, 2026
Closed
This was referenced Jun 19, 2026
Closed
Closed
Closed
@zenglingbiao
zenglingbiao deleted the fix/issue-91947-memory-search-timeout-config branch June 22, 2026 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: memory-core Extension: memory-core merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

memory_search 工具硬编码 15s timeout 不够 qmd query 跑完，建议可配置

1 participant

@zenglingbiao