Skip to content

fix(memory-core): add actionable remediation hints for memory search embedding errors#75951

Closed
QihongRuan wants to merge 2 commits into
openclaw:mainfrom
QihongRuan:fix/memory-search-error-hints-v3
Closed

fix(memory-core): add actionable remediation hints for memory search embedding errors#75951
QihongRuan wants to merge 2 commits into
openclaw:mainfrom
QihongRuan:fix/memory-search-error-hints-v3

Conversation

@QihongRuan

@QihongRuan QihongRuan commented May 2, 2026

Copy link
Copy Markdown

Summary

Fresh port of #55684 onto current main (the source branch was 16k commits behind and unmergeable). Adds actionable remediation hints when memory_search or openclaw memory status surfaces an embedding-provider failure, so leaked-key, quota, and invalid-auth errors stop printing as raw provider strings without recovery guidance.

Fixes #54912. Supersedes #55684 (closed-stale).

What changed

  • extensions/memory-core/src/tools.shared.ts — adds resolveEmbeddingErrorKind (classifies leaked / quota / invalid_key) and resolveEmbeddingErrorHint. buildMemorySearchUnavailableResult now prefers the kind-specific hint over the previous generic action text.
  • extensions/memory-core/src/cli.runtime.ts — adds resolveEmbeddingErrorRemediation with provider-specific env-var hints (GEMINI_API_KEY, OPENAI_API_KEY, VOYAGE_API_KEY) and emits a Fix line in runMemoryStatus after Embeddings error.
  • Tests — tools.test.ts and tools.citations.test.ts updated for the new quota action text; new tests for leaked-key and invalid-key paths.
  • CHANGELOG.md — single-line entry under Unreleased › Fixes (per repo policy and Codex P3 review on fix: add actionable remediation hints for memory search embedding errors #55684).

Codex review notes addressed (from #55684)

  • ✅ Branch is now on current main (no longer 16k commits diverged).
  • ✅ CHANGELOG entry added (the P3 finding from the prior Codex pass).
  • ✅ Focused tests for leaked-key, quota, and invalid-key classification added in tools.test.ts.

Test plan

  • pnpm exec oxfmt --check --threads=1 … — clean.
  • pnpm test extensions/memory-core/src/tools.test.ts extensions/memory-core/src/tools.citations.test.ts — 20/20 pass (2 new tests included).
  • pnpm test extensions/memory-core/src/cli.test.ts — 48/48 pass.
  • pnpm changed:lanes --base upstream/main reports only extension production + extension test; CHANGELOG is doc-only.
  • Verified the pnpm tsgo:extensions errors in src/secrets/runtime-web-tools* and extensions/google/web-search-provider.test.ts are pre-existing on upstream/main (unrelated to this change).

@clawsweeper

clawsweeper Bot commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep this PR open because current main still lacks leaked-key and invalid-key remediation for memory search embedding failures, but the branch is not merge-ready until the contributor adds real behavior proof, refreshes the conflict against current main, and removes the release-owned changelog edit.

Root-cause cluster
Relationship: canonical
Canonical: #75951
Summary: This PR is the current open implementation candidate for leaked-key and invalid-key memory-search remediation; earlier same-purpose PRs are closed unmerged, while node:sqlite and timeout diagnostics are adjacent same-helper work.

Members:

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

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

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

Yes, by source inspection. Current main routes leaked/invalid non-quota embedding failures to generic memory-search guidance and openclaw memory status only prints Embeddings error; I did not run a live provider failure because this read-only review should not use real provider secrets.

Is this the best way to solve the issue?

No, not merge-ready as submitted. Memory-core is the right layer, but the best fix needs a current-main conflict refresh, preservation of adjacent unavailable-result behavior, removal of the release-owned changelog edit, and contributor real behavior proof.

Security review:

Security review cleared: The diff is limited to memory-core diagnostic text, tests, and one changelog line, with no dependency, workflow, permission, download, or credential-storage change.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-05-02T04:36:31Z, is older than 60 days, and the latest review rated it F.
  • proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • vincentkoc: Opened the merged replacement PR that added current-main node:sqlite unavailable guidance in the same helper and tests this PR now conflicts with. (role: recent adjacent contributor; confidence: high; commits: 19627c7dd9b6, 3662b0765875; files: extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.test.ts)
  • rrrrrredy: Authored the source PR that classified missing node:sqlite errors and preserved quota/generic/override behavior later carried forward by the merged replacement. (role: source contributor for adjacent behavior; confidence: medium; commits: c4d709234fb2, 10e3b0db1d43, 3662b0765875; files: extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.test.ts)
  • vignesh07: CONTRIBUTING.md lists Vignesh for Memory/QMD, which is the affected memory-core area. (role: listed area contact; confidence: medium; files: CONTRIBUTING.md, extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/cli.runtime.ts)

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

@QihongRuan
QihongRuan force-pushed the fix/memory-search-error-hints-v3 branch from 85b1e3d to b797705 Compare May 2, 2026 05:01
@QihongRuan
QihongRuan force-pushed the fix/memory-search-error-hints-v3 branch from b797705 to f7e85fa Compare May 2, 2026 05:02
Tesla-20 added 2 commits May 2, 2026 00:23
@Tesla-20
fix(memory-core): add actionable remediation hints for embedding errors
b3a98eb 
Classify embedding-provider failures (leaked key, quota/rate-limit, invalid/unauthorized
key) and surface step-by-step recovery hints in memory_search unavailable payloads and
the openclaw memory status Fix line. Replaces the generic 'Check embedding provider
configuration' action with provider-aware guidance.

Fixes openclaw#54912.
@Tesla-20
fix(memory-core): direct leaked-key remediation to auth-profile rotation
5c3b25d 
Auth profiles resolve before env-var fallback, so suggesting GEMINI_API_KEY/
OPENAI_API_KEY/VOYAGE_API_KEY env-only rotation can leave a stale profile
winning. Drop env-var hints; point to `openclaw configure` plus gateway
restart. Addresses Codex P2 finding on openclaw#75951.
@QihongRuan
QihongRuan force-pushed the fix/memory-search-error-hints-v3 branch from f7e85fa to 5c3b25d Compare May 2, 2026 07:24
@clawsweeper clawsweeper Bot mentioned this pull request May 2, 2026
Closed
4 tasks
@QihongRuan

QihongRuan commented May 3, 2026

Copy link
Copy Markdown
Author

Heads up for reviewers: the 3 red CI checks (checks-node-core-runtime-infra, checks-node-agentic-commands-agent-channel, and the checks-node-core rollup) are not introduced by this PR. They fail with:

No "loadPluginRegistrySnapshotWithMetadata" export is defined on the "../plugins/plugin-registry.js" mock.

…in src/config/zod-schema.providers.lazy-runtime.test.ts and src/commands/channel-setup/workspace-shadow-bypass.test.ts — both src/ core files, no overlap with the extensions/memory-core/ surface this PR touches. Looks like a real export was added on main without updating the corresponding vi.mock in those tests.

Locally, pnpm test for the touched memory-core files (tools.test.ts, tools.citations.test.ts, cli.test.ts) is green: 68/68. Happy to rebase again once main settles.

@barnacle-openclaw

barnacle-openclaw Bot commented May 30, 2026

Copy link
Copy Markdown

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

@barnacle-openclaw barnacle-openclaw Bot added the stale Marked as stale due to inactivity label May 30, 2026
@clawsweeper clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label May 30, 2026
@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed stale Marked as stale due to inactivity labels May 31, 2026
@anyech anyech mentioned this pull request Jun 13, 2026
Closed
@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. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. 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. labels Jun 14, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 22, 2026
Closed
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jun 22, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 22, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 1, 2026
@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jul 5, 2026
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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

memory_search silently fails with leaked API key — no actionable recovery guidance

2 participants

@QihongRuan @Tesla-20