Skip to content

fix(memory): honor configured qmd search timeouts#92065

Closed
arkyu2077 wants to merge 1 commit into
openclaw:mainfrom
arkyu2077:pr-factory-91947
Closed

fix(memory): honor configured qmd search timeouts#92065
arkyu2077 wants to merge 1 commit into
openclaw:mainfrom
arkyu2077:pr-factory-91947

Conversation

@arkyu2077

Copy link
Copy Markdown
Contributor

Summary\n- derive the memory_search tool deadline from the configured QMD timeout when the qmd backend is active\n- keep a bounded wrapper overhead so hung searches still fail cleanly\n- add regression coverage for slow successful QMD searches and timeout behavior\n\n## Testing\n- node scripts/run-vitest.mjs extensions/memory-core/src/tools.test.ts\n\nFixes #91947

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 11, 2026
@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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close this PR as superseded: the underlying QMD memory_search timeout bug is still real, but this branch uses the rejected fixed-overhead timeout shape and is now replaced by the canonical issue plus the newer manager-owned candidate PR with real QMD proof.

Root-cause cluster
Relationship: superseded
Canonical: #91947
Summary: This PR is an older conflicting implementation attempt for the canonical configured-QMD-timeout mismatch; the active candidate fix is the newer open manager-owned timeout PR with real QMD proof.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Keep #91947 as the canonical tracker and review #95347, or another manager-owned whole-search budget fix, instead of this branch.

So I’m closing this here and keeping the remaining discussion on #91947 and #95347.

Review details

Best possible solution:

Keep #91947 as the canonical tracker and review #95347, or another manager-owned whole-search budget fix, instead of this branch.

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

Yes. Current main and v2026.6.9 wrap memory_search in a fixed 15s deadline while the linked issue shows QMD query work exceeding that budget with a larger configured QMD timeout.

Is this the best way to solve the issue?

No. This PR is not the best fix because a single QMD command timeout plus 5s does not cover QMD's whole search lifecycle; the safer replacement is the manager-owned budget in #95347.

Security review:

Security review cleared: No concrete security or supply-chain concern found; the diff only changes memory-core timeout logic and tests without dependency, workflow, secret, package-resolution, or downloaded-code changes.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: Commit 9e2bd8b added the fixed memory_search deadline wrapper this PR changes. (role: introduced timeout wrapper behavior; confidence: high; commits: 9e2bd8b2f7eb; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.test.ts)
  • dreamhunter2333: Commit 8d72cb9 added AbortSignal propagation through the same memory_search deadline path and SDK search contract. (role: recent timeout-path contributor; confidence: high; commits: 8d72cb9401e5; files: extensions/memory-core/src/tools.ts, packages/memory-host-sdk/src/host/types.ts)
  • TurboTheTurtle: Commit ddacb7b changed transient QMD manager lifecycle handling in tools.ts and qmd-manager.ts, adjacent to this timeout fix path. (role: recent QMD lifecycle contributor; confidence: medium; commits: ddacb7ba39d4; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts)
  • vincentkoc: Recent blame carries the affected wrapper through current main, and a member review on the related PR closed the same fixed-overhead approach as unsafe. (role: recent reviewer and area contributor; confidence: medium; commits: 63fdc57b3ac2, 5707038e6c5a; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts)
  • osolmaz: Commit 0dbf174 touched QMD query/rerank config and docs related to the timeout contract reused by this PR. (role: recent QMD config contributor; confidence: medium; commits: 0dbf17471b41; files: extensions/memory-core/src/memory/qmd-manager.ts, packages/memory-host-sdk/src/host/backend-config.ts, docs/reference/memory-config.md)

Codex review notes: model internal, reasoning high; reviewed against 6daf9307e0b9.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 22, 2026
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 merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant