Skip to content

fix(memory): attribute corpus=all timeouts to the slow branch instead of the provider#92647

Closed
Bartok9 wants to merge 1 commit into
openclaw:mainfrom
Bartok9:fix/92633-memory-search-timeout-attribution
Closed

fix(memory): attribute corpus=all timeouts to the slow branch instead of the provider#92647
Bartok9 wants to merge 1 commit into
openclaw:mainfrom
Bartok9:fix/92633-memory-search-timeout-attribution

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • memory_search with corpus=all could 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.
  • The warning now reflects that the tool timed out and names the slow branch (memory/session vs wiki/supplement), so users stop chasing a healthy provider config.

Motivation

Closes #92633.

The tool-level deadline wrapper (runMemorySearchToolWithDeadline) returns error: "memory_search timed out after 15s", which is funneled through buildMemorySearchUnavailableResult. That helper unconditionally attributed any non-quota failure to an embedding/provider error and told users to "Check embedding provider configuration". For a corpus=all aggregation stall this is misleading: the provider probe, corpus=memory, corpus=sessions, and corpus=wiki all 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:

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 existing corpus=all stall-in-wiki / stall-in-memory cases now asserting accurate attribution).
  • oxlint + oxfmt --check clean on all changed files.
  • tsgo extensions test project type-check: clean.

Did NOT change

  • Quota (insufficient_quota/429) and non-timeout provider-error messages are byte-identical to before.
  • The 15s deadline value and abort behavior are untouched — this is attribution only, not a timeout-tuning change.

Real behavior proof

  • Behavior or issue addressed: A corpus=all deadline 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 the debug block carries timedOut: true plus the offending phase.
  • Real environment tested: Ran the shipped, patched extensions/memory-core/src/tools.shared.ts module directly through the OpenClaw repo's node --import tsx runtime on macOS (arm64, Node 25), driving the live buildMemorySearchUnavailableResult export with the exact deadline string the tool wrapper emits (memory_search timed out after 8s) and the phase value that the corpus=all path threads in from failedUnavailablePhase. This is the real function the tool calls — not a stub or re-implementation.
  • Exact steps or command run after this patch:
    node --import tsx extensions/memory-core/.proof-driver.mts
    
    where the driver imports the real export and invokes it for the wiki/supplement slow branch, the memory/session slow branch, and a quota error.
  • Evidence after fix: verbatim runtime stdout captured from the patched module:
    === corpus=all, slow branch = wiki/supplement (phase: supplement) ===
    {
      "results": [],
      "disabled": true,
      "unavailable": true,
      "error": "memory_search timed out after 8s",
      "warning": "Memory search timed out before completing (slow branch: wiki/supplement).",
      "action": "Retry memory_search; if timeouts persist, narrow the corpus (e.g. corpus=memory) or check embedding/provider latency.",
      "debug": {
        "warning": "Memory search timed out before completing (slow branch: wiki/supplement).",
        "error": "memory_search timed out after 8s",
        "timedOut": true,
        "phase": "supplement"
      }
    }
    
    === corpus=all, slow branch = memory/session (phase: memory) ===
    {
      "results": [],
      "disabled": true,
      "unavailable": true,
      "error": "memory_search timed out after 8s",
      "warning": "Memory search timed out before completing (slow branch: memory/session).",
      "action": "Retry memory_search; if timeouts persist, narrow the corpus (e.g. corpus=memory) or check embedding/provider latency.",
      "debug": {
        "warning": "Memory search timed out before completing (slow branch: memory/session).",
        "error": "memory_search timed out after 8s",
        "timedOut": true,
        "phase": "memory"
      }
    }
    
    === quota error is still attributed to the provider (unchanged) ===
    {
      "results": [],
      "disabled": true,
      "unavailable": true,
      "error": "embedding request failed: insufficient_quota",
      "warning": "Memory search is unavailable because the embedding provider quota is exhausted.",
      "action": "Top up or switch embedding provider, then retry memory_search.",
      "debug": {
        "warning": "Memory search is unavailable because the embedding provider quota is exhausted.",
        "error": "embedding request failed: insufficient_quota"
      }
    }
    
  • Observed result after fix: The timed-out corpus=all outcome no longer blames the embedding provider. The runtime warning now reads "Memory search timed out before completing (slow branch: wiki/supplement)" (and the memory/session variant for the other branch), and the debug payload carries timedOut: true and the slow phase. 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.
  • What was not tested: A live end-to-end stall against a real embedding backend was not forced (reproducing a true 15s aggregation timeout on demand is impractical), so the deadline string was supplied exactly as the production wrapper emits it and fed through the real, unmodified attribution function. The full corpus=all orchestration (provider probe + per-corpus search + merge) is exercised by the committed extension suite rather than this runtime harness.

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: S 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

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed June 24, 2026, 5:41 AM ET / 09:41 UTC.

Summary
The PR changes memory-core memory_search unavailable results so tool-deadline timeouts can name the active memory/session or wiki/supplement branch and add timeout debug fields.

PR surface: Source +35, Tests +17. Total +52 across 5 files.

Reproducibility: yes. source-level. PR head matches any non-quota timed out after error, while current main emits provider embedding watchdog messages with that phrase and routes memory-phase search errors through the changed helper with a phase.

Review metrics: none identified.

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

Root-cause cluster
Relationship: partial_overlap
Canonical: #92633
Summary: This PR addresses the timeout-attribution/debug part of the canonical all-corpus memory_search timeout issue; the open concurrency PR addresses the additive-latency root cause.

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:

  • [P2] Restrict timeout matching to the exact tool-deadline string and add provider-timeout regression coverage.
  • Coordinate or narrow the PR body closing reference for the broader linked issue.

Risk before merge

Maintainer options:

  1. Fix The Timeout Classifier (recommended)
    Match only the tool-level memory_search timed out after ... wrapper and add provider-timeout regression coverage so diagnostics do not drift.
  2. Coordinate Issue Closure
    Update the closing wording or get maintainer confirmation so this attribution PR does not prematurely close the broader all-corpus timeout report while the concurrency PR remains open.
  3. Accept Diagnostic Drift
    Maintainers could intentionally accept the broader timeout wording, but provider watchdog failures would become harder for users to diagnose.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Restrict `buildMemorySearchUnavailableResult` timeout detection to the exact `memory_search timed out after ...` tool-deadline wrapper string, and add or update regression coverage proving provider embedding timeout messages such as `memory embeddings query timed out after 60s` still return embedding/provider guidance even when a phase is supplied.

Next step before merge

  • [P2] A focused repair can tighten the timeout classifier and add provider-timeout coverage; linked issue closure scope still needs maintainer or author coordination before merge.

Security
Cleared: The diff only changes memory-core TypeScript runtime/tests and does not add dependency, workflow, secret, package, or code-execution surface.

Review findings

  • [P2] Match only the memory_search deadline string — extensions/memory-core/src/tools.shared.ts:128
Review details

Best possible solution:

Restrict timeout attribution to the exact memory_search timed out after ... tool-deadline wrapper, keep provider-timeout guidance intact, add regression coverage, and coordinate linked issue closure with the concurrency PR.

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

Yes, source-level. PR head matches any non-quota timed out after error, while current main emits provider embedding watchdog messages with that phrase and routes memory-phase search errors through the changed helper with a phase.

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:

  • [P2] Match only the memory_search deadline string — extensions/memory-core/src/tools.shared.ts:128
    The new classifier treats any non-quota reason containing timed out after as a tool deadline. Current provider embedding watchdogs throw messages like memory embeddings query timed out after ..., and those errors can flow through this helper with a memory phase, so provider watchdog failures would be reported as memory/session branch timeouts instead of provider diagnostics. Match the exact memory_search timed out after ... wrapper and add provider-timeout coverage.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority memory_search diagnostics bugfix with limited blast radius in memory-core.
  • merge-risk: 🚨 other: The diff can misclassify provider watchdog timeouts and can close a broader linked issue before all requested behavior is handled.
  • 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 (terminal): The PR body includes terminal stdout from a patched node --import tsx run importing the real helper and showing intended timeout and quota payloads; the remaining blocker is classifier correctness, not proof absence.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal stdout from a patched node --import tsx run importing the real helper and showing intended timeout and quota payloads; the remaining blocker is classifier correctness, not proof absence.
Evidence reviewed

PR surface:

Source +35, Tests +17. Total +52 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 42 7 +35
Tests 2 29 12 +17
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 71 19 +52

Acceptance criteria:

  • [P2] node scripts/run-vitest.mjs run 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/manager-embedding-policy.test.ts.
  • [P1] corepack pnpm exec oxfmt --check 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/src/tools.test-helpers.ts.
  • [P1] node scripts/run-tsgo.mjs -p tsconfig.extensions.json --noEmit.

What I checked:

Likely related people:

  • dreamhunter2333: Authored the merged memory_search deadline/abort work that defines the memory_search timed out after ... wrapper and related timeout tests. (role: deadline behavior introducer; confidence: high; commits: 8d72cb9401e5, 0ca0a89fa92f, a36dc328fe5f; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.test.ts, extensions/memory-core/src/memory/manager-embedding-timeout.test.ts)
  • vincentkoc: Introduced the memory-core wiki corpus bridge used by corpus=all and also merged the deadline PR, making this a strong routing signal for both affected surfaces. (role: wiki corpus feature-history owner; confidence: high; commits: 2f72363984b9, 4d6cada25a42, 2cb200a0ad00; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.shared.ts, extensions/memory-wiki/index.ts)
  • Peter Steinberger: Recent memory-core history includes fail-open work around embedding recall stalls on the same search/provider-timeout surface. (role: recent memory search contributor; confidence: medium; commits: 9e2bd8b2f7eb; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.citations.test.ts, extensions/memory-core/src/memory/manager-embedding-ops.ts)
  • TurboTheTurtle: Merged PR 92639 recently touched nearby memory_search transient qmd flow and tests shortly before this timeout report cluster. (role: recent adjacent contributor; confidence: medium; commits: ddacb7ba39d4, 42d7eb4a52cc, 8a55faf9cf9e; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.citations.test.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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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
@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. labels Jun 13, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 13, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 19, 2026
… 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
@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 15, 2026
@openclaw-barnacle

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

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: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S 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 corpus=all times out while individual corpora succeed

1 participant