Skip to content

fix(memory): abort search embeddings on tool timeout#91770

Open
ai-hpc wants to merge 2 commits into
openclaw:mainfrom
ai-hpc:ai-hpc/fix-memory-search-timeout-abort
Open

fix(memory): abort search embeddings on tool timeout#91770
ai-hpc wants to merge 2 commits into
openclaw:mainfrom
ai-hpc:ai-hpc/fix-memory-search-timeout-abort

Conversation

@ai-hpc

@ai-hpc ai-hpc commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #91718 by making the memory_search tool timeout cancel the underlying builtin memory embedding search instead of only racing the outer tool promise.

  • Adds an optional AbortSignal to the memory search manager options contract.
  • Aborts the in-flight memory search when the 15s tool deadline fires.
  • Threads the signal into builtin query embedding and the provider fetch operation.
  • Stops embedding retries after caller cancellation.
  • Keeps caller-aborted searches from activating the configured fallback provider after cancellation.

Verification

  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test extensions/memory-core/src/tools.test.ts extensions/memory-core/src/tools.citations.test.ts extensions/memory-core/src/memory/manager-embedding-timeout.test.ts extensions/memory-core/src/memory/search-manager.test.ts
  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=0 pnpm test extensions/memory-core/src/memory/index.test.ts extensions/memory-core/src/memory/manager-embedding-timeout.test.ts
  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree pnpm tsgo:core:test
  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree pnpm tsgo:extensions:test
  • pnpm format:check -- extensions/memory-core/src/tools.ts extensions/memory-core/src/tools.test.ts extensions/memory-core/src/memory-tool-manager-mock.ts extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/search-manager.ts extensions/memory-core/src/memory/qmd-manager.ts extensions/memory-core/src/memory/manager-embedding-ops.ts extensions/memory-core/src/memory/manager-embedding-timeout.test.ts packages/memory-host-sdk/src/host/types.ts packages/memory-host-sdk/src/engine-storage.ts src/plugin-sdk/memory-core-host-engine-storage.ts
  • pnpm format:check -- extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/index.test.ts
  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree timeout 300s node scripts/run-oxlint.mjs src/plugin-sdk/memory-core-host-engine-storage.ts
  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree timeout 300s node scripts/run-oxlint.mjs extensions/memory-core/src/tools.ts extensions/memory-core/src/tools.test.ts extensions/memory-core/src/memory-tool-manager-mock.ts extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/search-manager.ts extensions/memory-core/src/memory/qmd-manager.ts extensions/memory-core/src/memory/manager-embedding-ops.ts extensions/memory-core/src/memory/manager-embedding-timeout.test.ts packages/memory-host-sdk/src/host/types.ts packages/memory-host-sdk/src/engine-storage.ts
  • OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-oxlint.mjs extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/index.test.ts
  • node --import tsx /tmp/openclaw-91718-proof.mjs
  • git diff --check

Real behavior proof

behavior: memory_search timeout cancellation now reaches the builtin embedding provider operation.

environment: Ubuntu/Linux local OpenClaw source checkout on Node 22.22.2, branch ai-hpc/fix-memory-search-timeout-abort.

steps:

  1. Started a local delayed HTTP endpoint on 127.0.0.1.
  2. Ran the real runEmbeddingOperationWithTimeout implementation with a parent AbortSignal representing the memory_search 15s deadline.
  3. Started a provider-style fetch against the delayed endpoint using the operation signal.
  4. Aborted the parent signal with memory_search timed out after 15s.
  5. Verified the operation rejected quickly, the provider signal was aborted with the same reason, and the HTTP connection closed before the delayed response.

evidence:

{
  "result": "aborted",
  "errorMessage": "memory_search timed out after 15s",
  "elapsedMs": 219,
  "operationSignalAborted": true,
  "operationSignalReceivedParentReason": true,
  "serverRequestSeen": true,
  "serverConnectionClosedBeforeResponse": true
}

observedResult: the caller timeout now cancels the active provider fetch path instead of leaving it to run in the background.

notTested: live external embedding vendor endpoint. The local loopback proof covers the same AbortSignal-to-fetch cancellation path without using real provider credentials.

@clawsweeper

clawsweeper Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 30, 2026, 5:07 PM ET / 21:07 UTC.

Summary
The PR makes memory_search caller cancellation settle embedding operations sooner, avoids fallback activation after caller abort, and exports a named MemorySearchOptions type through memory host SDK/plugin SDK surfaces.

PR surface: Source -7, Tests +100. Total +93 across 10 files.

Reproducibility: yes. Current main source shows the lower embedding timeout helper still waits on the provider operation or watchdog when a provider ignores AbortSignal, and the PR body supplies loopback live output for the abort-to-fetch path.

Review metrics: 1 noteworthy metric.

  • Public SDK surface: 1 named type/export added. MemorySearchOptions would become part of a public Plugin SDK subpath, so maintainers should notice and baseline or remove the API change before merge.

Stored data model
Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/memory/manager-embedding-ops.ts, vector/embedding metadata: extensions/memory-core/src/memory/index.test.ts, vector/embedding metadata: extensions/memory-core/src/memory/manager-embedding-ops.ts, vector/embedding metadata: extensions/memory-core/src/memory/manager-embedding-timeout.test.ts, vector/embedding metadata: extensions/memory-core/src/memory/manager.ts, vector/embedding metadata: extensions/memory-core/src/memory/qmd-manager.ts, and 3 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #91718
Summary: This PR is a candidate refinement for the memory_search timeout issue; merged PR #91742 already fixed the central tool-deadline signal flow, while this branch adds lower-helper provider-ignores-signal behavior and public type export.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • Resolve the merge conflict against current main.
  • Get maintainer acceptance for the public MemorySearchOptions type/export or remove it.
  • Regenerate the Plugin SDK API baseline if the export stays.

Risk before merge

  • [P1] The branch is currently dirty/conflicting against main, so maintainers cannot evaluate the actual merge result until it is rebased or refreshed.
  • [P1] The PR adds a named public Plugin SDK type/export without the generated API baseline update.
  • [P2] The PR changes embedding provider cancellation behavior when a caller aborts an in-flight memory search operation, which is provider/fallback behavior maintainers should intentionally accept.

Maintainer options:

  1. Refresh then accept and baseline the public type
    If maintainers want MemorySearchOptions as public SDK API, rebase the branch and commit the regenerated Plugin SDK API baseline before merge.
  2. Refresh with a private options type
    If the named type is only implementation cleanup, resolve conflicts by keeping the cancellation refinement local and avoiding a new public SDK export.
  3. Pause behind the merged fix
    Because the central linked issue is already fixed on main, maintainers can pause or close this branch if the extra provider-abort behavior is not worth the API and conflict cost.

Next step before merge

  • [P2] The remaining action is maintainer API/compatibility judgment plus conflict resolution; the baseline change is mechanical, but the branch also needs a human decision on the public API and provider behavior.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes memory cancellation plumbing, exported types, and tests without dependency, workflow, secret, or package execution changes.

Review findings

  • [P2] Regenerate the SDK API baseline hash — src/plugin-sdk/memory-core-host-engine-storage.ts:65
Review details

Best possible solution:

Refresh the branch, then land only the lower-helper provider-ignores-signal refinement if maintainers accept the behavior and either baseline the public SDK type or keep it private.

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

Yes. Current main source shows the lower embedding timeout helper still waits on the provider operation or watchdog when a provider ignores AbortSignal, and the PR body supplies loopback live output for the abort-to-fetch path.

Is this the best way to solve the issue?

Mostly yes. Settling parent aborts in runEmbeddingOperationWithTimeout is the narrow runtime refinement, but the public MemorySearchOptions export and conflict state need maintainer handling.

Full review comments:

  • [P2] Regenerate the SDK API baseline hash — src/plugin-sdk/memory-core-host-engine-storage.ts:65
    This export adds MemorySearchOptions to a public Plugin SDK subpath, but the generated Plugin SDK API baseline hash is unchanged. Run pnpm plugin-sdk:api:gen and commit docs/.generated/plugin-sdk-api-baseline.sha256, or keep the type private if the export is not intentional.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded memory_search resource-retention refinement with limited blast radius.
  • merge-risk: 🚨 compatibility: The diff adds a named public MemorySearchOptions export through a Plugin SDK subpath while the generated API baseline is unchanged.
  • merge-risk: 🚨 auth-provider: The diff changes embedding provider cancellation behavior when a caller aborts an in-flight memory search operation.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes after-fix loopback live output showing parent abort propagation to a provider-style fetch path and connection close before response.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix loopback live output showing parent abort propagation to a provider-style fetch path and connection close before response.
Evidence reviewed

PR surface:

Source -7, Tests +100. Total +93 across 10 files.

View PR surface stats
Area Files Added Removed Net
Source 7 67 74 -7
Tests 3 101 1 +100
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 10 168 75 +93

What I checked:

  • Live PR state blocks cleanup close: GitHub reports author_association=MEMBER, head fb0df59, mergeable=false, and mergeable_state=dirty. (fb0df5953dc6)
  • Current main has the central tool deadline signal path: runMemorySearchToolWithDeadline creates an AbortController, aborts the losing task on timeout, and the memory_search call passes the deadline signal into search options. (extensions/memory-core/src/tools.ts:126, b885c81479d0)
  • Current main still lacks immediate parent-abort settling in the lower helper: runEmbeddingOperationWithTimeout merges the caller signal with AbortSignal.any, but only races the provider operation against the watchdog timeout, so a provider that ignores AbortSignal can keep the helper pending until the watchdog or operation settles. (extensions/memory-core/src/memory/manager-embedding-ops.ts:171, b885c81479d0)
  • PR head adds the residual lower-helper cancellation refinement: The PR head adds parentAbortPromise/abortFromParent and races parent abort alongside the provider operation and watchdog timeout. (extensions/memory-core/src/memory/manager-embedding-ops.ts:181, fb0df5953dc6)
  • PR head adds a public Plugin SDK type export: The PR head exports MemorySearchOptions through src/plugin-sdk/memory-core-host-engine-storage.ts and defines it in the memory host SDK types. (src/plugin-sdk/memory-core-host-engine-storage.ts:65, fb0df5953dc6)
  • SDK boundary policy requires aligned API checks: Scoped Plugin SDK guidance says public subpath changes must keep docs, entrypoints, package exports, and API baseline/export checks aligned. (src/plugin-sdk/AGENTS.md:81, b885c81479d0)

Likely related people:

  • dreamhunter2333: Authored the merged related PR that implemented current main's central memory_search deadline cancellation path. (role: related fix author; confidence: high; commits: 0ca0a89fa92f, a36dc328fe5f; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/manager.ts, extensions/memory-core/src/memory/manager-embedding-ops.ts)
  • vincentkoc: Merged the related central fix and authored follow-up commits preserving builtin cancellation and fallback behavior. (role: related fix merger and verifier; confidence: high; commits: 4d6cada25a42, 2cb200a0ad00, 8d72cb9401e5; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/manager.ts, extensions/memory-core/src/memory/manager-embedding-ops.ts)
  • Peter Steinberger: History shows memory host SDK extraction and memory embedding state helper refactors across the SDK and manager boundary this PR touches. (role: adjacent memory SDK refactor owner; confidence: medium; commits: eebce9e9c7cb, d806682f782e; files: packages/memory-host-sdk/src/host/types.ts, extensions/memory-core/src/memory/manager-embedding-ops.ts)
  • Agustin Rivera: Current-main blame for the timeout helper, embedding helper, and memory host SDK contract points to broad commit 4aa0751, though feature-specific routing is stronger through the related memory PR history. (role: recent current-main carrier; confidence: low; commits: 4aa07513fe89; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/manager-embedding-ops.ts, packages/memory-host-sdk/src/host/types.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.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T16:35:06.679Z sha fb0df59 :: found issues before merge. :: [P2] Regenerate the SDK API baseline hash

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. 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. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 9, 2026
@ai-hpc
ai-hpc force-pushed the ai-hpc/fix-memory-search-timeout-abort branch from 690ce57 to fb0df59 Compare June 14, 2026 00:52
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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. labels Jun 14, 2026
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 21, 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: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: M stale Marked as stale due to inactivity status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

memory_search tool-level timeout orphans background embedding work

1 participant