fix(memory): abort search embeddings on tool timeout#91770
Conversation
|
Codex review: found issues before merge. Reviewed June 30, 2026, 5:07 PM ET / 21:07 UTC. Summary 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.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b885c81479d0. Label changesLabel justifications:
Evidence reviewedPR surface: Source -7, Tests +100. Total +93 across 10 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
690ce57 to
fb0df59
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
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.
AbortSignalto the memory search manager options contract.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.tsOPENCLAW_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.tsOPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree pnpm tsgo:core:testOPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree pnpm tsgo:extensions:testpnpm 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.tspnpm format:check -- extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/index.test.tsOPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree timeout 300s node scripts/run-oxlint.mjs src/plugin-sdk/memory-core-host-engine-storage.tsOPENCLAW_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.tsOPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-oxlint.mjs extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/index.test.tsnode --import tsx /tmp/openclaw-91718-proof.mjsgit diff --checkReal 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:
127.0.0.1.runEmbeddingOperationWithTimeoutimplementation with a parent AbortSignal representing the memory_search 15s deadline.fetchagainst the delayed endpoint using the operation signal.memory_search timed out after 15s.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.