Skip to content

fix(memory-core): clarify memory_search tool timeouts#92632

Closed
anyech wants to merge 1 commit into
openclaw:mainfrom
anyech:fix/memory-search-tool-timeout-classification
Closed

fix(memory-core): clarify memory_search tool timeouts#92632
anyech wants to merge 1 commit into
openclaw:mainfrom
anyech:fix/memory-search-tool-timeout-classification

Conversation

@anyech

@anyech anyech commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the memory_search tool timeout path so tool-deadline failures are no longer reported as embedding-provider failures, and makes the deadline configurable for slower hosts / larger indexes.

What changed:

  • add plugins.entries.memory-core.config.tools.memorySearch.timeoutMs (1000-120000 ms) for the memory_search tool hard deadline;
  • classify memory_search timed out after ... as a tool deadline failure instead of an embedding/provider error;
  • return partial primary-memory results for corpus="all" when the supplemental/wiki corpus stalls after memory hits are already available;
  • document the new timeout knob and partial-result behavior.

Relationship to adjacent work

This is adjacent to, but not a duplicate of:

Behavior addressed

memory_search could hit the hardcoded 15s tool deadline and surface:

Memory search is unavailable due to an embedding/provider error.

That message is misleading when the embedding provider is healthy and the failure is the wrapper deadline. Broad corpus="all" searches could also discard usable primary memory results if the supplemental/wiki phase stalled.

Real setup tested

Isolated source checkout on current origin/main. I did not use any live user Gateway/runtime/config for proof.

Exact steps or command run after this patch

node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-memory.config.ts extensions/memory-core/src/tools.test.ts extensions/memory-core/src/tools.citations.test.ts extensions/memory-core/src/config.test.ts
corepack pnpm exec oxfmt --check extensions/memory-core/src/tools.ts extensions/memory-core/src/tools.shared.ts extensions/memory-core/src/tools.test.ts extensions/memory-core/src/tools.citations.test.ts extensions/memory-core/openclaw.plugin.json docs/concepts/memory-search.md
node scripts/run-tsgo.mjs -p tsconfig.extensions.json --noEmit

Evidence after fix

  • Targeted Vitest: 3 files, 36 tests passed.
  • Formatting check passed for touched files.
  • Extension type gate passed.

Observed result after fix

  • Timeout tests now report Memory search timed out before the tool deadline. instead of the generic embedding/provider warning.
  • A configured tools.memorySearch.timeoutMs: 5000 produces memory_search timed out after 5s and aborts the active search signal.
  • corpus="all" returns partial: true and primary memory hits when the supplemental corpus stalls after primary memory hits are available.

What was not tested

Live Gateway behavior on a production/user instance was intentionally not tested.

OpenClaw Assistant
Fix memory_search tool timeout handling
543f2b9 
Classify memory_search tool deadline failures separately from embedding provider errors, add a configurable memory-core tool timeout, and return partial corpus=all results when a supplemental corpus stalls after primary memory results are available.
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: memory-core Extension: memory-core size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 13, 2026
@clawsweeper

clawsweeper Bot commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 13, 2026, 4:21 AM ET / 08:21 UTC.

Summary
The PR adds configurable memory_search tool timeout handling, clearer timeout classification, partial corpus=all results when supplements stall, tests, and concept docs.

PR surface: Source +139, Tests +66, Docs +8. Total +213 across 6 files.

Reproducibility: yes. Source inspection on current main shows a fixed 15s memory_search wrapper and generic provider-error classification for timeout strings; I did not run a live Gateway/QMD repro in this read-only review.

Review metrics: 1 noteworthy metric.

  • Config surfaces added: 1 added: tools.memorySearch.timeoutMs. A new public plugin config key should have schema, reference docs, tests, and upgrade expectations checked before merge.

Stored data model
Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/tools.citations.test.ts, unknown-data-model-change: extensions/memory-core/src/tools.test.ts, unknown-data-model-change: extensions/memory-core/src/tools.ts, vector/embedding metadata: docs/concepts/memory-search.md, vector/embedding metadata: extensions/memory-core/src/tools.citations.test.ts, vector/embedding metadata: extensions/memory-core/src/tools.shared.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

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:

  • [P2] Post redacted terminal output, logs, or a recording from a real OpenClaw memory_search invocation showing the after-fix timeout behavior.
  • [P1] Fix the lint error at the partial-result debug spread.
  • [P2] Document the new timeout key in the memory configuration reference and add manifest schema coverage.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies targeted tests plus format/type output and explicitly says no live Gateway/runtime/config proof was used, so contributor real behavior proof is not yet sufficient. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Real behavior proof is still missing; the PR body reports only targeted tests, formatting, and type checks, and explicitly says no live Gateway/runtime/config proof was used.
  • [P1] The new public config key needs reference docs and schema-test coverage before it becomes a stable operator-facing contract.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the explicit memory-core tool timeout direction, fix the lint error, align the config reference/test coverage, and require redacted real memory_search runtime proof before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining blockers include contributor real behavior proof and public config-contract review, so this should stay in human PR review rather than a repair-only lane.

Security
Cleared: The diff changes memory-core runtime logic, tests, docs, and plugin JSON schema only; it adds no dependencies, scripts, credentials, permissions, artifact downloads, or supply-chain execution path.

Review findings

  • [P2] Remove the useless debug spread fallback — extensions/memory-core/src/tools.ts:251
  • [P2] Add the timeout key to the config reference — extensions/memory-core/openclaw.plugin.json:33-46
Review details

Best possible solution:

Keep the explicit memory-core tool timeout direction, fix the lint error, align the config reference/test coverage, and require redacted real memory_search runtime proof before merge.

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

Yes. Source inspection on current main shows a fixed 15s memory_search wrapper and generic provider-error classification for timeout strings; I did not run a live Gateway/QMD repro in this read-only review.

Is this the best way to solve the issue?

No as submitted. The explicit memory-core timeout key is a better direction than borrowing raw QMD settings, but the lint, config reference/test, and real-proof gaps should be fixed before merge.

Full review comments:

  • [P2] Remove the useless debug spread fallback — extensions/memory-core/src/tools.ts:251
    CI is failing on this changed spread with unicorn(no-useless-fallback-in-spread) in both lint jobs. Object spread already treats undefined as a no-op, so use ...params.debug or build the debug object another way before merge.
    Confidence: 0.99
  • [P2] Add the timeout key to the config reference — extensions/memory-core/openclaw.plugin.json:33-46
    This adds a public plugins.entries.memory-core.config.tools.memorySearch.timeoutMs schema key, but the memory config reference still documents memory-core plugin config only for Dreaming. Add reference coverage and a manifest-schema acceptance case so the default, range, and path do not drift from the implementation.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7bd533a80e9d.

Label changes

Label changes:

  • add P2: The PR addresses a normal memory_search reliability and diagnostics bug with a limited memory-core blast radius.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies targeted tests plus format/type output and explicitly says no live Gateway/runtime/config proof was used, so contributor real behavior proof is not yet sufficient. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: The PR addresses a normal memory_search reliability and diagnostics bug with a limited memory-core blast radius.
  • 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 body supplies targeted tests plus format/type output and explicitly says no live Gateway/runtime/config proof was used, so contributor real behavior proof is not yet sufficient. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +139, Tests +66, Docs +8. Total +213 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 143 4 +139
Tests 2 80 14 +66
Docs 1 8 0 +8
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 231 18 +213

What I checked:

Likely related people:

  • steipete: GitHub commit history ties Peter Steinberger to the fail-open memory_search timeout wrapper and several adjacent memory tool validation changes. (role: introduced timeout behavior and recent area contributor; confidence: high; commits: 9e2bd8b2f7eb, 4ac6bb196492, e9cca2d1efdf; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.shared.ts)
  • dreamhunter2333: GitHub history shows recent work threading AbortSignal through the memory_search timeout path, which is directly adjacent to this PR's deadline handling. (role: recent timeout-path contributor; confidence: medium; commits: 8d72cb9401e5; files: extensions/memory-core/src/tools.ts)
  • osolmaz: Recent memory provider, QMD, and memory configuration reference commits make this person relevant for timeout/config alignment and documentation review. (role: recent memory config/docs contributor; confidence: medium; commits: a4b4fed41287, 0aea58ab66d4, 0dbf17471b41; files: docs/reference/memory-config.md, extensions/memory-core/src/tools.ts, 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. labels Jun 13, 2026
This was referenced Jun 13, 2026
Open
Closed
@anyech

anyech commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Closing this voluntarily. Recent maintainer feedback made clear I should avoid keeping implementation PRs open when the change is feature/config/hardening-oriented rather than a maintainer-requested bugfix.

This overlaps the existing #91947 / #91958 memory_search timeout lane and currently has failing checks, so keeping another implementation PR open is counterproductive. Happy to revisit if maintainers want this exact implementation or a narrower patch.

@anyech anyech closed this Jun 14, 2026
This was referenced Jun 17, 2026
Closed
Closed
Closed
Closed
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

docs Improvements or additions to documentation extensions: memory-core Extension: memory-core P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@anyech