Skip to content

fix(memory): gracefully degrade when embedding provider is unregistered#91862

Closed
xydt-tanshanshan wants to merge 1 commit into
openclaw:mainfrom
xydt-tanshanshan:fix/memory-index-metadata-missing-91778
Closed

fix(memory): gracefully degrade when embedding provider is unregistered#91862
xydt-tanshanshan wants to merge 1 commit into
openclaw:mainfrom
xydt-tanshanshan:fix/memory-index-metadata-missing-91778

Conversation

@xydt-tanshanshan

@xydt-tanshanshan xydt-tanshanshan commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

When a configured embedding provider (e.g. openai) has no registered adapter — typically because the provider plugin is not installed — ensureProviderInitialized() previously let the Unknown memory embedding provider error propagate, crashing the CLI on openclaw memory status --index.

Now the error is caught and the manager gracefully degrades to FTS-only mode, setting providerUnavailableReason so that openclaw memory status still works and shows the real problem instead of a stack trace.

This addresses the unregistered-provider crash path reported in #91778. The broader index-metadata/vector-search failure tracked in that issue remains outside this diff.

Change Type

  • Bug fix

Scope

  • Memory / storage

Linked Issue

Motivation

Issue #91778 reports memory_search vector search broken since v2026.6.1. Users with provider: "openai-compatible" or provider: "openai" configured but without the corresponding provider plugin installed get a CLI crash on openclaw memory status --index, preventing any diagnosis or recovery.

Root cause: createEmbeddingProvider() → getAdapter() throws Error("Unknown memory embedding provider: <id>") when the provider plugin is not installed. This error propagates through loadProviderResult() → ensureProviderInitialized() → probeEmbeddingAvailability() → CLI crash. The user never gets to see what is wrong.

This PR only fixes the unregistered-provider crash path. The remaining index-metadata and vector-search recovery behavior in #91778 is outside the scope of this diff.

Changes

File: extensions/memory-core/src/memory/manager.ts

In ensureProviderInitialized(), catch Unknown memory embedding provider: errors specifically and degrade to FTS-only mode instead of propagating the error:

  • Detect the error by checking message.startsWith("Unknown memory embedding provider:")
  • Call applyProviderResult({ provider: null, requestedProvider, providerUnavailableReason: message }) to set the correct FTS-only lifecycle state
  • Other provider initialization errors (timeouts, auth failures, rate limits) still propagate as before

File: extensions/memory-core/src/memory/manager.unknown-provider-degradation.test.ts (new)

2 tests covering the degradation path:

  1. probeEmbeddingAvailability() returns { ok: false, error } instead of throwing
  2. status().custom.providerState is { mode: "fts-only", reason: "Unknown memory embedding provider: ..." }

No new imports, no config changes, no default surface changes.

Real Behavior Proof

  • Behavior addressed: openclaw memory status --index crashes with Unknown memory embedding provider: <id> when the configured embedding provider plugin is not installed, preventing any diagnosis or recovery.
  • Real environment tested: Linux (RHEL), Node.js 22.22, local checkout on branch fix/memory-index-metadata-missing-91778 at commit df80fe7, dev build via node scripts/run-node.mjs.
  • Exact steps or command run after this patch: Set agents.defaults.memorySearch.provider to "nonexistent-embedding-provider" in ~/.openclaw/openclaw.json, then run node scripts/run-node.mjs memory status --index
  • Evidence after fix: Terminal output from the current PR head (df80fe7) showing CLI no longer crashes when an unregistered embedding provider is configured. The manager degrades to FTS-only mode with a clear error message instead of a stack trace. Copied live output from node scripts/run-node.mjs memory status --index with provider: "nonexistent-embedding-provider":
Memory Search (main)
Provider: nonexistent-embedding-provider (requested: nonexistent-embedding-provider)
Sources: memory
Indexed: 0/49 files · 0 chunks
Dirty: yes
Embeddings: unavailable
Embeddings error: Unknown memory embedding provider: nonexistent-embedding-provider
Index identity: index metadata is missing
Vector search: paused until memory is rebuilt
Fix: Run: openclaw memory status --index --agent main
Semantic vectors: unavailable
FTS: ready
Index error: Memory sync unavailable: embedding provider "nonexistent-embedding-provider" is configured but unavailable. Reason: Unknown memory embedding provider: nonexistent-embedding-provider. lifecycle={"mode":"fts-only","reason":"Unknown memory embedding provider: nonexistent-embedding-provider","attemptedProviderId":"nonexistent-embedding-provider"}

Before-fix comparison (installed build without the openai provider plugin, provider: "openai" configured):

$ openclaw memory status --index
[openclaw] Failed to start CLI: Error: Unknown memory embedding provider: openai
    at getAdapter (embeddings.ts:49:11)
    at createEmbeddingProvider (embeddings.ts:138:26)
    at MemoryIndexManager.loadProviderResult (manager.ts:162:58)
    → CLI exits with error code 1, no status output
  • Observed result after fix: ensureProviderInitialized() catches the Unknown memory embedding provider error and degrades to FTS-only mode. The CLI no longer crashes; users see Embeddings: unavailable, Embeddings error: Unknown memory embedding provider: ..., Index identity: index metadata is missing, and Semantic vectors: unavailable. FTS search still works. The lifecycle state is correctly set to {"mode":"fts-only","reason":"Unknown memory embedding provider: ...","attemptedProviderId":"..."}.
  • What was not tested: Full end-to-end vector indexing with a working embedding provider after the fix. The assertRequiredProviderAvailable path in sync is intentionally unchanged: explicit provider requests that fail should still block sync from silently downgrading an existing vector index.

Risks

  • Low risk: The catch is narrow — only Unknown memory embedding provider: errors are caught and degraded. All other provider init errors (auth, network, timeout) still propagate as before.
  • Additive: The degradation only activates when getAdapter() throws for an unregistered provider. Previously this was a guaranteed crash; now it is a graceful FTS-only fallback with a clear error message.
  • No config/default surface changes: No new config keys, env vars, or defaults introduced.
  • Scope limitation: This PR only addresses the unregistered-provider crash path. The broader index-metadata and vector-search recovery tracked in [P0] memory_search cassé — index metadata is missing depuis v2026.6.1, reproduit sur v2026.6.5 #91778 remains open and is not closed by merging this PR.

@xydt-tanshanshan
fix(memory): gracefully degrade when embedding provider is unregistered
df80fe7 
When a configured embedding provider (e.g. 'openai') has no registered
adapter—typically because the provider plugin is not installed—
ensureProviderInitialized() previously let the 'Unknown memory embedding
provider' error propagate, crashing the CLI on .

Now the error is caught and the manager gracefully degrades to FTS-only
mode, setting providerUnavailableReason so that  still
works and shows the real problem instead of a stack trace.

Refs openclaw#91778
@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. 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. proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 10, 2026
@clawsweeper

clawsweeper Bot commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 10, 2026, 2:53 AM ET / 06:53 UTC.

Summary
The PR catches unregistered memory embedding provider initialization errors in memory-core, records an FTS-only provider lifecycle, and adds focused probe/status degradation tests.

PR surface: Source +15, Tests +105. Total +120 across 2 files.

Reproducibility: yes. at source level: current main's adapter lookup throws for an unregistered provider and ensureProviderInitialized rethrows that into probe/status callers. I did not run the CLI in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Next step before merge

  • No automated repair is needed because this review found no actionable line-level defect; remaining action is ordinary maintainer review, CI, and linked-issue scope management.

Security
Cleared: The diff changes memory-core runtime error handling and tests only; it does not touch dependencies, lockfiles, scripts, secrets, publishing, or other supply-chain surfaces.

Review details

Best possible solution:

Land the narrow diagnostic degradation after ordinary maintainer review and CI, while keeping #91778 open for the remaining index-metadata and vector-search recovery work.

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

Yes at source level: current main's adapter lookup throws for an unregistered provider and ensureProviderInitialized rethrows that into probe/status callers. I did not run the CLI in this read-only review.

Is this the best way to solve the issue?

Yes for the narrow crash path: the manager is the right boundary to translate provider initialization failure into lifecycle state, and the existing sync/search required-provider guards still prevent silent vector-index downgrade. It is not a complete fix for the broader linked memory_search issue.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against a3d5e5bc7247.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix terminal output from the PR head showing memory status --index renders embeddings and index diagnostics instead of a startup stack trace, which is sufficient real CLI proof for this non-visual change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix terminal output from the PR head showing memory status --index renders embeddings and index diagnostics instead of a startup stack trace, which is sufficient real CLI proof for this non-visual change.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: The PR fixes a real user-facing memory status/probe crash, but the patch is narrow and limited to memory-core diagnostics.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix terminal output from the PR head showing memory status --index renders embeddings and index diagnostics instead of a startup stack trace, which is sufficient real CLI proof for this non-visual change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix terminal output from the PR head showing memory status --index renders embeddings and index diagnostics instead of a startup stack trace, which is sufficient real CLI proof for this non-visual change.
Evidence reviewed

PR surface:

Source +15, Tests +105. Total +120 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 16 1 +15
Tests 1 105 0 +105
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 121 1 +120

What I checked:

Likely related people:

  • osolmaz: Recent merged memory provider commits changed required-provider unavailable behavior and local llama.cpp provider ownership, both central to this PR's provider initialization boundary. (role: feature-history contributor; confidence: high; commits: 0aea58ab66d4, 31371101678d; files: extensions/memory-core/src/memory/manager.ts, extensions/memory-core/src/memory/embeddings.ts)
  • vincentkoc: Recent commit metadata for memory manager changes shows Vincent Koc as committer on adjacent FTS/model identity behavior in the same manager file. (role: recent area contributor; confidence: medium; commits: e3ef136bca85; files: extensions/memory-core/src/memory/manager.ts)
  • shakkernerd: Local blame in this shallow checkout attributes the current manager, sync, and embeddings files to the current main snapshot commit carried by Shakker. (role: recent area carrier; confidence: medium; commits: 440284f87955; files: extensions/memory-core/src/memory/manager.ts, extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/embeddings.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.

@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 10, 2026
@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. proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. and removed 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. labels Jun 10, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 10, 2026
Closed
This was referenced Jun 11, 2026
Merged
Closed
@xydt-tanshanshan

xydt-tanshanshan commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@vincentkoc this is the prerequisite for the FTS-only providerKey auto-fix (#92079). ensureProviderInitialized() currently throws on unknown providers instead of degrading gracefully. Mind taking a look?

@anyech anyech mentioned this pull request Jun 13, 2026
Closed
@xydt-tanshanshan

xydt-tanshanshan commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR since the linked issue #91778 was closed as not_planned. The project direction appears to prefer fail-fast on unknown providers over graceful degradation — a reasonable choice that avoids silently masking configuration errors. Dependent PR #92079 is also paused accordingly. Happy to revisit if the direction changes.

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

extensions: memory-core Extension: memory-core 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: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@xydt-tanshanshan