Skip to content

fix(memory-core): honor qmd search timeouts#92624

Closed
yu-xin-c wants to merge 1 commit into
openclaw:mainfrom
yu-xin-c:codex/memory-search-qmd-deadline
Closed

fix(memory-core): honor qmd search timeouts#92624
yu-xin-c wants to merge 1 commit into
openclaw:mainfrom
yu-xin-c:codex/memory-search-qmd-deadline

Conversation

@yu-xin-c

@yu-xin-c yu-xin-c commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Closes #91947

Summary

  • Derive the memory_search outer deadline from the canonical resolved QMD backend config when QMD is configured above the default 15s tool guard.
  • Keep builtin memory search, wiki-only search, and default QMD behavior on the existing 15s guard.
  • Add regression coverage for a slow-but-successful QMD search, a never-settling QMD search, and the canonical non-integer QMD timeout normalization path.

Real behavior proof

  • Behavior addressed: memory_search had a fixed 15s wrapper deadline, so QMD searches with memory.qmd.limits.timeoutMs set higher could still fail at 15s before QMD's configured search budget was honored.
  • Real environment tested: Local OpenClaw checkout on macOS arm64, branch codex/memory-search-qmd-deadline, running OpenClaw's real memory_search tool against the real QMD backend. QMD was executed through a temporary wrapper that delays only search/query/vsearch commands for 16s, then execs the live @tobilu/qmd CLI. This isolates the old outer-deadline failure without relying on a naturally slow reranker.
  • Exact steps or command run after this patch:
    cat >/tmp/openclaw-qmd-proof-wrapper.sh <<'SH'
    #!/usr/bin/env bash
    set -euo pipefail
    case "${1:-}" in
      search | query | vsearch)
        sleep "${OPENCLAW_QMD_PROOF_SLEEP_SECONDS:-16}"
        ;;
    esac
    exec npx -y @tobilu/qmd "$@"
    SH
    chmod +x /tmp/openclaw-qmd-proof-wrapper.sh
    
    OPENCLAW_QMD_PROOF_SLEEP_SECONDS=16 ./node_modules/.bin/tsx --conditions=development - <<'TS'
    import fs from "node:fs/promises";
    import os from "node:os";
    import path from "node:path";
    import { createMemorySearchTool } from "./extensions/memory-core/src/tools.ts";
    
    const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-qmd-proof-"));
    await fs.mkdir(path.join(workspace, "memory"), { recursive: true });
    await fs.writeFile(path.join(workspace, "MEMORY.md"), "# Memory\nOPENCLAW_QMD_TIMEOUT_SENTINEL_92624 proves memory_search can wait past fifteen seconds.\n", "utf8");
    await fs.writeFile(path.join(workspace, "memory", "qmd-proof.md"), "# QMD proof note\nA second OPENCLAW_QMD_TIMEOUT_SENTINEL_92624 note is indexed from memory/*.md.\n", "utf8");
    
    const cfg = {
      agents: { list: [{ id: "main", default: true, workspace }] },
      memory: {
        backend: "qmd",
        citations: "off",
        qmd: {
          command: "/tmp/openclaw-qmd-proof-wrapper.sh",
          includeDefaultMemory: true,
          searchMode: "search",
          update: { onBoot: true, interval: "5m" },
          limits: { maxResults: 4, timeoutMs: 60_000.5 },
        },
      },
    } as const;
    
    const tool = createMemorySearchTool({ config: cfg as any, agentId: "main", agentSessionKey: "agent:main:discord:dm:qmd-proof" });
    if (!tool) throw new Error("memory_search tool was not created");
    const started = Date.now();
    const result = await tool.execute("qmd-live-proof", { query: "OPENCLAW_QMD_TIMEOUT_SENTINEL_92624", maxResults: 4 });
    const elapsedMs = Date.now() - started;
    const details = result.details as any;
    console.log(JSON.stringify({ elapsedMs, disabled: details?.disabled ?? false, error: details?.error, debug: details?.debug, resultCount: details?.results?.length }, null, 2));
    if (elapsedMs <= 15_000) throw new Error(`expected >15s, got ${elapsedMs}ms`);
    if (details?.disabled || details?.error) throw new Error(`unavailable: ${details?.error}`);
    if (details?.debug?.backend !== "qmd") throw new Error(`expected qmd backend, got ${details?.debug?.backend}`);
    if (!Array.isArray(details?.results) || details.results.length === 0) throw new Error("expected qmd results");
    process.exit(0);
    TS
  • Evidence after fix:
    [memory] qmd watcher starting for agent "main" paths=2
    [memory] qmd manager initialized for agent "main" mode=full collections=2 durationMs=6864
    [memory] qmd watcher ready for agent "main" paths=2 durationMs=32
    [memory] qmd sync completed for agent "main" reason=boot durationMs=1160
    {
      "workspace": "$TMPDIR/openclaw-qmd-proof-qRvwoo",
      "qmdCommand": "/tmp/openclaw-qmd-proof-wrapper.sh",
      "qmdCli": "@tobilu/qmd 2.5.3 via npx -y",
      "configuredTimeoutMs": 60000.5,
      "expectedOuterDeadlineMs": 65000,
      "elapsedMs": 25535,
      "disabled": false,
      "debug": {
        "backend": "qmd",
        "configuredMode": "search",
        "effectiveMode": "search",
        "searchMs": 18563,
        "hits": 2
      },
      "resultCount": 2,
      "results": [
        { "path": "MEMORY.md", "source": "memory", "corpus": "memory" },
        { "path": "memory/qmd-proof.md", "source": "memory", "corpus": "memory" }
      ]
    }
    
  • Observed result after fix: The real memory_search tool completed successfully after 25.5s with QMD backend debug metadata and two live QMD results. This exceeds the old fixed 15s wrapper deadline and stays below the patched 65s wrapper deadline derived from canonical memory.qmd.limits.timeoutMs: 60000.5 plus bounded overhead.
  • What was not tested: I did not wait for a naturally slow reranker/model run; the proof uses a search-command delay wrapper that immediately delegates to the live @tobilu/qmd CLI so the outer memory_search deadline behavior is reproducible without external model latency.

Validation

node scripts/run-vitest.mjs extensions/memory-core/src/tools.test.ts      # 19 passed
node scripts/run-vitest.mjs extensions/memory-core/src/tools.citations.test.ts # 20 passed
./node_modules/.bin/oxfmt --check --threads=1 extensions/memory-core/src/tools.ts extensions/memory-core/src/tools.test.ts extensions/memory-core/src/memory-tool-manager-mock.ts
node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/memory-core/src/tools.ts extensions/memory-core/src/tools.test.ts extensions/memory-core/src/memory-tool-manager-mock.ts
pnpm tsgo:extensions:test
git diff --check

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

clawsweeper Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 13, 2026, 11:19 AM ET / 15:19 UTC.

Summary
The PR derives the memory_search outer deadline from resolved QMD backend timeout settings for active QMD searches and adds regression coverage for slow and stalled QMD paths.

PR surface: Source +60, Tests +90. Total +150 across 3 files.

Reproducibility: yes. Current main's fixed 15s outer deadline can fire before a QMD search using a higher configured memory.qmd.limits.timeoutMs completes; the linked issue's 46.7s reranking trace matches that source path.

Review metrics: 1 noteworthy metric.

  • Config surface changes: 0 added, 0 changed, 0 removed. The PR honors the existing memory.qmd.limits.timeoutMs contract instead of adding another public timeout knob.

Stored data model
Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/tools.test.ts, unknown-data-model-change: extensions/memory-core/src/tools.ts, vector/embedding metadata: extensions/memory-core/src/memory-tool-manager-mock.ts, vector/embedding metadata: extensions/memory-core/src/tools.test.ts, vector/embedding metadata: extensions/memory-core/src/tools.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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

  • [P2] Several open PRs target the same linked QMD timeout issue; maintainers should choose one canonical branch before landing overlapping memory_search deadline changes.

Maintainer options:

  1. Decide the mitigation before merge
    Land one canonical QMD timeout fix that derives the wrapper deadline from the resolved active QMD backend budget while preserving 15 seconds for non-QMD and wiki-only paths.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The branch looks review-ready, but overlapping open timeout PRs make the remaining action a maintainer canonical-branch choice rather than an automated code repair.

Security
Cleared: The diff changes memory-core timeout selection and tests only; it adds no dependency, workflow, credential, permission, artifact-download, or package-resolution surface.

Review details

Best possible solution:

Land one canonical QMD timeout fix that derives the wrapper deadline from the resolved active QMD backend budget while preserving 15 seconds for non-QMD and wiki-only paths.

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

Yes. Current main's fixed 15s outer deadline can fire before a QMD search using a higher configured memory.qmd.limits.timeoutMs completes; the linked issue's 46.7s reranking trace matches that source path.

Is this the best way to solve the issue?

Yes, this is the best fix shape among the inspected timeout PRs: it reuses canonical resolved QMD configuration, avoids a new config key, and keeps wiki-only, builtin, and default QMD paths on the existing 15s guard. Maintainers should still choose this or another branch as the canonical PR before merging overlapping fixes.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 45056a463ab2.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from the real memory_search tool against a delayed wrapper that delegates to live @tobilu/qmd and completes after the old 15s deadline with QMD results.

Label justifications:

  • P2: The PR addresses a focused memory-core reliability bug for slow QMD search configurations with limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live output from the real memory_search tool against a delayed wrapper that delegates to live @tobilu/qmd and completes after the old 15s deadline with QMD results.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from the real memory_search tool against a delayed wrapper that delegates to live @tobilu/qmd and completes after the old 15s deadline with QMD results.
Evidence reviewed

PR surface:

Source +60, Tests +90. Total +150 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 72 12 +60
Tests 1 90 0 +90
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 162 12 +150

What I checked:

Likely related people:

  • steipete: Live GitHub history shows this account authored the memory_search fail-open timeout wrapper and several adjacent memory tool validation/config-refresh changes. (role: introduced timeout behavior and frequent memory tool contributor; confidence: high; commits: 9e2bd8b2f7eb, 4ac6bb196492, 8a8cc8dc9fca; files: extensions/memory-core/src/tools.ts)
  • TurboTheTurtle: Merged PR history changed the same memory_search/QMD manager path for transient QMD mode, which this PR must preserve. (role: recent adjacent QMD lifecycle contributor; confidence: high; commits: ddacb7ba39d4, 9408380ae729; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts)
  • osolmaz: Recent history shows QMD query rerank, memory config reference, and memory index identity work overlapping the timeout contract reused here. (role: recent QMD config and docs contributor; confidence: medium; commits: 0dbf17471b41, 0aea58ab66d4, a4b4fed41287; files: docs/reference/memory-config.md, packages/memory-host-sdk/src/host/backend-config.ts, extensions/memory-core/src/memory/qmd-manager.ts)
  • dreamhunter2333: Recent history threaded AbortSignal through memory_search timeout behavior, directly adjacent to this PR's outer deadline handling. (role: recent timeout-path contributor; confidence: medium; commits: 8d72cb9401e5; files: extensions/memory-core/src/tools.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.

@yu-xin-c
yu-xin-c force-pushed the codex/memory-search-qmd-deadline branch from 7a1b29d to ca55d54 Compare June 13, 2026 07:37
@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. labels Jun 13, 2026
@yu-xin-c
yu-xin-c force-pushed the codex/memory-search-qmd-deadline branch from ca55d54 to 81f41f6 Compare June 13, 2026 07:52
@yu-xin-c

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 13, 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:

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 13, 2026
@yu-xin-c
yu-xin-c force-pushed the codex/memory-search-qmd-deadline branch from 81f41f6 to 1407fef Compare June 13, 2026 15:04
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jun 13, 2026
@yu-xin-c

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 13, 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:

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 13, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@vincentkoc vincentkoc self-assigned this Jun 13, 2026
@vincentkoc

Copy link
Copy Markdown
Member

maintainer deep review found the reported bug is real, but this PR's fix is not safe to land.

The proposed outer deadline is derived from one memory.qmd.limits.timeoutMs, while the QMD manager can execute multiple sequential bounded commands across collection groups, compatibility fallback, missing-collection repair, and empty-result sync/retry. A single QMD timeout plus 5s can still terminate valid configured searches. It also loads the memory runtime outside the deadline and extends supplement-only corpus=all calls during memory cooldown.

I am closing this PR rather than broadening the patch into a risky timeout rewrite. Issue #91947 remains open for a canonical fix: QMD should own and expose a defensible whole-search budget (or fully enforce its own finite lifecycle), while runtime loading and wiki supplements retain their separate bounded deadlines.

Reviewed against current main and the QMD manager command paths in extensions/memory-core/src/memory/qmd-manager.ts; autoreview also reproduced these three blocking gaps.

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 工具硬编码 15s timeout 不够 qmd query 跑完,建议可配置

2 participants