fix(memory): attribute corpus=all timeouts to the slow branch instead of the provider#92647
fix(memory): attribute corpus=all timeouts to the slow branch instead of the provider#92647Bartok9 wants to merge 1 commit into
Conversation
|
Codex review: needs changes before merge. Reviewed June 24, 2026, 5:41 AM ET / 09:41 UTC. Summary PR surface: Source +35, Tests +17. Total +52 across 5 files. Reproducibility: yes. source-level. PR head matches any non-quota Review metrics: none identified. 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:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Restrict timeout attribution to the exact Do we have a high-confidence way to reproduce the issue? Yes, source-level. PR head matches any non-quota Is this the best way to solve the issue? No as written. The helper/caller surface is the right narrow place for attribution, but the classifier needs exact tool-deadline matching so provider-originated timeouts keep provider guidance. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f65aca64fc37. Label changesLabel justifications:
Evidence reviewedPR surface: Source +35, Tests +17. Total +52 across 5 files. View PR surface stats
Acceptance criteria:
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
|
… of provider memory_search with corpus=all could time out (15s tool deadline) while the embedding provider probe and each individual corpus searched successfully. The deadline error was funneled through buildMemorySearchUnavailableResult, which unconditionally blamed an embedding/provider error and told users to check provider configuration -- misleading when the real problem is a slow aggregation branch. Detect the deadline timeout message and emit an accurate warning that names the slow branch (memory/session vs wiki/supplement) using the phase the tool already tracks, plus debug.timedOut/phase fields. Quota and genuine provider errors keep their existing messages. Closes openclaw#92633
792d804 to
ae517be
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
Summary
memory_searchwithcorpus=allcould hit the 15s tool deadline and return a "Memory search is unavailable due to an embedding/provider error" warning even when the embedding provider probe and every individual corpus searched successfully.memory/sessionvswiki/supplement), so users stop chasing a healthy provider config.Motivation
Closes #92633.
The tool-level deadline wrapper (
runMemorySearchToolWithDeadline) returnserror: "memory_search timed out after 15s", which is funneled throughbuildMemorySearchUnavailableResult. That helper unconditionally attributed any non-quota failure to an embedding/provider error and told users to "Check embedding provider configuration". For acorpus=allaggregation stall this is misleading: the provider probe,corpus=memory,corpus=sessions, andcorpus=wikiall succeed individually — only the combined path times out.The search tool already tracks which corpus branch was active when a failure occurred (
activeUnavailablePhase/failedUnavailablePhase). This change:buildMemorySearchUnavailableResultand emits a timeout-specific warning + action instead of the provider-error text (issue ask WA business, groups & office hours #3).debug.timedOut/debug.phase(issue ask Images not passed to Claude CLI - only path reference in text #4).Verification
node scripts/run-vitest.mjs run extensions/memory-core/src/tools.test.ts extensions/memory-core/src/tools.citations.test.ts— 34 passed (covers manager-setup timeout, memory-branch timeout + cooldown, abort-aware timeout, and the two existingcorpus=allstall-in-wiki / stall-in-memory cases now asserting accurate attribution).oxlint+oxfmt --checkclean on all changed files.Did NOT change
insufficient_quota/429) and non-timeout provider-error messages are byte-identical to before.Real behavior proof
corpus=alldeadline stall used to surface "Memory search is unavailable due to an embedding/provider error" and point users at embedding-provider config, even though the provider and each individual corpus were healthy. After this patch the runtime warning names the actual slow branch and thedebugblock carriestimedOut: trueplus the offendingphase.extensions/memory-core/src/tools.shared.tsmodule directly through the OpenClaw repo'snode --import tsxruntime on macOS (arm64, Node 25), driving the livebuildMemorySearchUnavailableResultexport with the exact deadline string the tool wrapper emits (memory_search timed out after 8s) and thephasevalue that thecorpus=allpath threads in fromfailedUnavailablePhase. This is the real function the tool calls — not a stub or re-implementation.corpus=alloutcome no longer blames the embedding provider. The runtimewarningnow reads "Memory search timed out before completing (slow branch: wiki/supplement)" (and thememory/sessionvariant for the other branch), and thedebugpayload carriestimedOut: trueand the slowphase. The genuine quota path is untouched and still attributes to the provider, confirming the change is scoped to deadline stalls. The driver file was removed after capturing this output; it was a throwaway harness, not committed.corpus=allorchestration (provider probe + per-corpus search + merge) is exercised by the committed extension suite rather than this runtime harness.