Skip to content

Deprecate memory-specific embedding provider registration#85072

Merged
mbelinky merged 1 commit into
mainfrom
mbelinky/memory-embedding-provider-deprecation
May 27, 2026
Merged

Deprecate memory-specific embedding provider registration#85072
mbelinky merged 1 commit into
mainfrom
mbelinky/memory-embedding-provider-deprecation

Conversation

@mbelinky
Copy link
Copy Markdown
Contributor

@mbelinky mbelinky commented May 21, 2026

Summary

  • Rebase the deprecation PR onto current main after feat(embeddings): add OpenAI-compatible core provider #85269 landed the generic embedding provider bridge and core openai-compatible provider.
  • Keep only the deprecation/compatibility work: the memory-specific registerMemoryEmbeddingProvider(...) path remains available, but is marked as deprecated compatibility.
  • Surface non-bundled legacy usage in plugin compatibility diagnostics, including runtime-only api.registerMemoryEmbeddingProvider(...) registrations that do not declare contracts.memoryEmbeddingProviders.
  • Add guardrails so new bundled plugins do not add more legacy memory embedding provider registrations.

Verification

Behavior addressed: old memory-specific embedding provider registration now remains compatible but visible as deprecated for plugin authors, and the previous review gap around runtime-only registrations is covered.

Real environment tested: local OpenClaw worktree on macOS.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs src/plugins/status.test.ts src/plugins/compat/registry.test.ts src/plugins/contracts/plugin-sdk-package-contract-guardrails.test.ts
git diff --check origin/main..HEAD
node_modules/.bin/oxfmt --check CHANGELOG.md docs/plugins/adding-capabilities.md docs/plugins/compatibility.md docs/plugins/manifest.md docs/plugins/sdk-migration.md docs/plugins/sdk-overview.md docs/plugins/sdk-provider-plugins.md docs/plugins/sdk-subpaths.md src/plugin-sdk/memory-core-host-engine-embeddings.ts src/plugins/compat/registry.test.ts src/plugins/compat/registry.ts src/plugins/contracts/plugin-sdk-package-contract-guardrails.test.ts src/plugins/status.test-helpers.ts src/plugins/status.test.ts src/plugins/status.ts

Evidence after fix: focused Vitest passed 3 files / 54 tests; git diff --check passed; oxfmt passed all 15 touched files.

Observed result after fix: external/workspace plugins using the legacy manifest contract or runtime api.registerMemoryEmbeddingProvider(...) receive a deprecated-memory-embedding-provider-api compatibility notice, bundled legacy registrations remain suppressed from user warnings, and new bundled legacy registrations are blocked by guardrail tests.

What was not tested: full build/full changed gate; this PR is now a narrow deprecation/docs/diagnostics cleanup on top of the already-merged #85269 provider stack.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: S maintainer Maintainer-authored PR labels May 21, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 21, 2026

Codex review: needs real behavior proof before merge. Reviewed May 27, 2026, 9:19 AM ET / 13:19 UTC.

Summary
The PR marks memory-specific embedding provider registration as deprecated compatibility, adds plugin-inspect compatibility notices for external legacy usage, adds bundled guardrail tests, and updates plugin SDK docs.

PR surface: Source +58, Tests +146, Docs +18. Total +222 across 15 files.

Reproducibility: yes. for the review finding: source inspection of the PR head shows a CHANGELOG.md entry while the root policy says normal PRs must not edit the release-owned changelog. The runtime diagnostic behavior is source-reviewable in the PR diff but still lacks current real behavior proof.

Review metrics: 1 noteworthy metric.

  • Public plugin deprecations: 1 added. The PR starts a compatibility-warning and removal-window record for a public plugin SDK/manifest surface, which needs maintainer API approval before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Remove the CHANGELOG.md entry and keep release-note context in the PR body or squash message.
  • Add current-head redacted terminal/log proof from openclaw plugins inspect or an equivalent real plugin load path showing the deprecation notice.
  • Get maintainer confirmation for the public plugin SDK deprecation warning and removal dates.

Proof guidance:
Needs real behavior proof before merge: The PR body lists focused tests and formatting checks but no current after-fix CLI/plugin-inspect output; add redacted terminal output, logs, or a terminal screenshot and update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • This deprecates a public plugin SDK and manifest path, so maintainers need to explicitly accept the warning behavior and the 2026-08-21 removal window before merge.
  • The PR body lists focused tests and formatting checks, but no current after-fix CLI/plugin-inspect output or equivalent runtime proof for the deprecation notice path.
  • The PR currently edits release-owned CHANGELOG.md, which should be removed before a normal PR merge.

Maintainer options:

  1. Clean up before maintainer approval (recommended)
    Remove the CHANGELOG.md entry, add current-head redacted CLI/plugin-inspect proof, and have the assigned maintainer confirm the deprecation warning and removal dates before merge.
  2. Hold the deprecation window
    If the plugin SDK removal timing is not settled, pause this PR and keep the generic embedding-provider replacement available without starting diagnostics yet.
  3. Accept the compatibility warning
    Maintainers may intentionally accept the warning/removal schedule once proof and changelog cleanup are complete because the old API remains wired during the migration window.

Next step before merge
Maintainer review and contributor real behavior proof are needed before merge because this changes a public plugin SDK deprecation schedule; automation cannot supply the contributor's real setup proof.

Security
Cleared: The diff is SDK/docs/test diagnostic work and does not add dependencies, workflow permissions, package-resolution changes, secrets handling, or downloaded code execution.

Review findings

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:10
Review details

Best possible solution:

Land a narrow deprecation follow-up after removing the changelog edit, adding current real inspect proof, and confirming the plugin SDK deprecation schedule with the assigned maintainer.

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

Yes for the review finding: source inspection of the PR head shows a CHANGELOG.md entry while the root policy says normal PRs must not edit the release-owned changelog. The runtime diagnostic behavior is source-reviewable in the PR diff but still lacks current real behavior proof.

Is this the best way to solve the issue?

Not yet. The deprecation direction is maintainable and now covers runtime-only registrations, but the merge-ready path is to remove the changelog edit, provide current real inspect proof, and get maintainer approval for the public plugin API deprecation window.

Full review comments:

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:10
    CHANGELOG.md is release-owned in this repo, and normal PRs should keep release-note context in the PR body or commit metadata instead. Please drop this Unreleased entry so release generation remains the only source that rewrites the changelog.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: This is low-urgency SDK/docs compatibility cleanup, not an active user-breaking runtime regression.
  • merge-risk: 🚨 compatibility: The PR marks a public plugin API and manifest contract as deprecated and introduces migration diagnostics for external plugin authors.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists focused tests and formatting checks but no current after-fix CLI/plugin-inspect output; add redacted terminal output, logs, or a terminal screenshot and update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +58, Tests +146, Docs +18. Total +222 across 15 files.

View PR surface stats
Area Files Added Removed Net
Source 4 64 6 +58
Tests 3 146 0 +146
Docs 8 40 22 +18
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 15 250 28 +222

What I checked:

  • Root policy read and applied: Root AGENTS.md requires full policy review and says CHANGELOG.md is release-owned; normal PRs should keep release-note context in the PR body or commit metadata instead of editing the changelog. (AGENTS.md:191, 4a8d89f8b53a)
  • Scoped plugin SDK policy read: The scoped plugin SDK guide identifies this area as the public contract between plugins and core, so deprecating the memory-specific registrar is compatibility-sensitive. (src/plugin-sdk/AGENTS.md:1, 4a8d89f8b53a)
  • Release-owned changelog edit in PR: The PR head adds an Unreleased CHANGELOG.md entry for this PR, which conflicts with the repository's release-owned changelog policy for normal PRs. (CHANGELOG.md:10, d4178c8eba01)
  • Deprecation diagnostic implementation in PR: The PR head adds compatibility warnings when an external plugin uses manifest-declared or runtime-only memory embedding provider registration, while suppressing bundled plugin migration debt. (src/plugins/status.ts:141, d4178c8eba01)
  • Deprecation schedule added in PR: The PR adds a compatibility record with warning and removal dates for deprecated memory-specific embedding provider API usage. (src/plugins/compat/registry.ts:54, d4178c8eba01)
  • Guardrail tests added in PR: The PR adds allowlists for existing bundled legacy usage and a guardrail that fails if new bundled plugins add memory-specific embedding provider API or manifest registrations. (src/plugins/contracts/plugin-sdk-package-contract-guardrails.test.ts:57, d4178c8eba01)

Likely related people:

  • dutifulbob: Related merged provider work added the generic embedding provider contract and core OpenAI-compatible provider that this deprecation points plugin authors toward. (role: generic embedding provider feature owner; confidence: high; commits: 4d89e00c5093, ae4806ed9adf; files: src/plugins/types.ts, src/plugins/embedding-providers.ts, src/plugins/openai-compatible-embedding-provider.ts)
  • mbelinky: The current PR author is also credited in the merged core embedding-provider history and authored related bridge/deprecation work in the cluster, so they are connected beyond this proposal alone. (role: embedding bridge contributor and reviewer; confidence: high; commits: 4d89e00c5093, d4178c8eba01; files: extensions/memory-core/src/memory/embeddings.ts, src/plugins/status.ts, src/plugins/compat/registry.ts)
  • Andy Ye: Current-main blame for the existing plugin status and SDK registrar lines points to this recent commit, making them a useful routing candidate for current source context even though the commit topic was adjacent. (role: recent current-main line contributor; confidence: medium; commits: aa0a29099fc5; files: src/plugins/status.ts, src/plugins/types.ts, src/plugin-sdk/memory-core-host-engine-embeddings.ts)
  • vincentkoc: The live PR is assigned to this handle, which is relevant for maintainer follow-up on the public SDK deprecation decision. (role: assigned reviewer; confidence: medium; files: src/plugins/compat/registry.ts, src/plugin-sdk/memory-core-host-engine-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.

@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. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 21, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 21, 2026

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@mbelinky mbelinky force-pushed the mbelinky/memory-embedding-provider-deprecation branch from e03b9e8 to 55472fe Compare May 21, 2026 19:45
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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 May 21, 2026
@vincentkoc vincentkoc self-assigned this May 22, 2026
@vincentkoc vincentkoc force-pushed the mbelinky/memory-generic-embedding-provider-bridge branch from 3e858f0 to bfa2494 Compare May 22, 2026 01:37
@vincentkoc vincentkoc force-pushed the mbelinky/memory-embedding-provider-deprecation branch from 55472fe to b25a168 Compare May 22, 2026 01:37
@mbelinky mbelinky force-pushed the mbelinky/memory-embedding-provider-deprecation branch from b25a168 to d4178c8 Compare May 27, 2026 13:03
@mbelinky mbelinky changed the base branch from mbelinky/memory-generic-embedding-provider-bridge to main May 27, 2026 13:04
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord channel: imessage Channel integration: imessage channel: signal Channel integration: signal channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: whatsapp-web Channel integration: whatsapp-web labels May 27, 2026
@mbelinky
Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 27, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@mbelinky
Copy link
Copy Markdown
Contributor Author

Merged via squash.

Thanks @mbelinky!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants