feat(memory): pluggable system prompt section for memory plugins#40126
feat(memory): pluggable system prompt section for memory plugins#40126jalehman merged 12 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR makes the memory plugin's system prompt section pluggable by introducing a module-level registry ( Key changes:
Confidence Score: 4/5
Last reviewed commit: 01b4d60 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01b4d60bac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35703c4bfc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
37676db to
1afb607
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1afb6075dc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb28f63a4b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
bd409d1 to
e1281f6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1281f6cac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jalehman
left a comment
There was a problem hiding this comment.
Changes Requested
Thank you for this contribution! We've reviewed this PR and have some feedback before it can move forward.
Reset the registered memory prompt section when clearing plugin loader cache
clearPluginLoaderCache() currently clears the plugin registry cache but leaves the active memory prompt section builder in place. That means stale memory-plugin instructions can survive a cache reset and still affect later system prompts. Please make the cache-clear path fully reset the memory prompt section state as well, and extend the tests to cover that reset behavior directly.
Rebase onto main and restore a green CI run
GitHub currently reports merge conflicts against main, and the PR is also failing check plus checks (node, channels, pnpm test:channels). Please rebase or merge main into the branch, resolve the conflict, and update the branch until the required checks are green so we can review the final behavior in a current branch state.
Once these are addressed, we'll re-review. Feel free to ask questions if anything is unclear.
e1281f6 to
a77b9b1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a77b9b152d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46528f3e2e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
46528f3 to
c3b2c5f
Compare
|
I now updated I have a couple of points: First, I have problems with CI even after rebasing to main: check, node test, and bun test are green but there are still errors. They seem to be unrelated to this PR, but main is fully green. I tried rebasing a couple of times. Can you please check this? Second, I would like to raise a more general point: How about memory plugin would not support plugin reconfiguration during the runtime? Due to its nature (the internal funtionality assumes continuatity), memory-plugin is in a special position that it would make sense, to require that is does not change when openclaw is running. What do think about this? |
c3b2c5f to
80cc0b9
Compare
jalehman
left a comment
There was a problem hiding this comment.
Rebased onto main and pushed merge-prep updates to the PR branch.
What changed:
- restored memory prompt builder state across
activate:falsesnapshot loads - rolled back newly-registered memory prompt state when plugin
register()fails - exported the new
openclaw/plugin-sdk/memory-coresubpath so plugin-sdk boundary checks pass
Local verification on the rebased branch:
pnpm test -- src/plugins/loader.test.ts src/memory/prompt-section.test.ts extensions/memory-core/index.test.tspnpm checkpnpm build
I am intentionally not adding a try/catch wrapper around buildMemoryPromptSection(): for this surface, a broken memory-plugin prompt builder should still fail loudly during prompt assembly rather than silently degrade behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80cc0b9c07
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return buildMemoryPromptSection({ | ||
| availableTools: params.availableTools, | ||
| citationsMode: params.citationsMode, | ||
| }); |
There was a problem hiding this comment.
Preserve default Memory Recall section for prompt-only callers
This change makes buildAgentSystemPrompt() depend on a prior registerMemoryPromptSection() side effect, but there are still in-repo callers that synthesize memory_search/memory_get tool sets without loading plugins first (for example src/agents/prompt-composition-scenarios.ts:39-99). In those flows the default memory-core instructions now disappear unless some earlier code happened to populate the global builder, so prompt-composition fixtures become ambient-state-dependent instead of matching the default memory-core slot behavior.
Useful? React with 👍 / 👎.
80cc0b9 to
9fde296
Compare
Regeneration-Prompt: | Refresh prepare for PR openclaw#40126 after origin/main advanced again during push. The refreshed base exposed a build failure unrelated to the memory prompt feature: src/memory/embeddings-openai.ts declared DEFAULT_OPENAI_EMBEDDING_MODEL twice after a recent mainline fix landed. Remove only the duplicate export so the rebased prep branch keeps the shared embedding default alias that memory sync expects while restoring a green build.
ac6ac5e to
9cf4316
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cf4316fd9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return buildMemoryPromptSection({ | ||
| availableTools: params.availableTools, | ||
| citationsMode: params.citationsMode, |
There was a problem hiding this comment.
Clone availableTools before delegating to memory plugins
buildMemoryPromptSection() receives the live availableTools Set here, and buildAgentSystemPrompt() reuses that same Set later for sections like buildMessagingSection(). For third-party memory plugins, a perfectly plausible implementation is to mutate the set while narrowing it to memory-related tools (for example, deleting non-memory entries); that would silently remove unrelated prompt guidance such as the message section from the same request. Passing a cloned/read-only set into the memory callback would keep prompt assembly deterministic even when plugin code manipulates its inputs.
Useful? React with 👍 / 👎.
1be06c2 to
e953198
Compare
Add registerMemoryPromptSection to the plugin API so the active memory plugin provides its own system prompt builder callback. The built-in memory-core plugin registers the existing Memory Recall section via this new API. When no memory plugin is active (or a plugin does not register a builder), the memory section is simply omitted from the system prompt. This lets third-party memory plugins inject their own guidance at the same position in the prompt.
Throw if a non-memory plugin attempts to register a memory prompt section builder, enforcing the exclusive slot constraint at the API level.
e953198 to
5228d19
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5228d1937f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -1,11 +1,38 @@ | |||
| import { definePluginEntry } from "openclaw/plugin-sdk/core"; | |||
| import type { MemoryPromptSectionBuilder } from "openclaw/plugin-sdk/memory-core"; | |||
There was a problem hiding this comment.
Route memory-core SDK types through a local barrel
extensions/memory-core/index.ts now imports openclaw/plugin-sdk/memory-core from inside the extensions/memory-core package itself. The repo explicitly forbids this pattern in AGENTS.md ("inside an extension package, do not import that same extension via openclaw/plugin-sdk/<extension> from production files") because it pulls the package back through the SDK alias/export path instead of the local source graph. That makes future lazy-load/build refactors prone to duplicate-module or ineffective-dynamic-import regressions for this extension surface, which is why other extensions use local api.ts/runtime-api.ts barrels instead.
Useful? React with 👍 / 👎.
* main: (516 commits) fix: use content hash for memory flush dedup instead of compactionCount (openclaw#30115) (openclaw#34222) fix(tts): add matrix to VOICE_BUBBLE_CHANNELS (openclaw#37080) feat(memory): pluggable system prompt section for memory plugins (openclaw#40126) fix: detect nvm services from installed command (openclaw#51146) fix: handle Linux nvm CA env before startup (openclaw#51146) (thanks @GodsBoy) refactor: route Telegram runtime through plugin sdk (openclaw#51772) refactor: route iMessage runtime through plugin sdk (openclaw#51770) refactor: route Slack runtime through plugin sdk (openclaw#51766) refactor(doctor): extract provider and shared config helpers (openclaw#51753) Fix Discord `/codex_resume` picker expiration (openclaw#51260) fix(ci): remove duplicate embedding default export fix(ci): restore embedding defaults and plugin boundaries fix: compaction safeguard summary budget (openclaw#27727) web UI: fix context notice using accumulated inputTokens instead of prompt snapshot (openclaw#51721) fix(status): skip cold-start status probes refactor(doctor): extract telegram provider warnings (openclaw#51704) fix(telegram): default fresh setups to mention-gated groups docs(changelog): note telegram doctor first-run guidance fix(doctor): add telegram first-run guidance fix(doctor): suppress telegram fresh-install group warning ...
…nclaw#40126) Merged via squash. Prepared head SHA: 5228d19 Co-authored-by: jarimustonen <[email protected]> Co-authored-by: jalehman <[email protected]> Reviewed-by: @jalehman
…nclaw#40126) Merged via squash. Prepared head SHA: 5228d19 Co-authored-by: jarimustonen <[email protected]> Co-authored-by: jalehman <[email protected]> Reviewed-by: @jalehman
…nclaw#40126) Merged via squash. Prepared head SHA: 5228d19 Co-authored-by: jarimustonen <[email protected]> Co-authored-by: jalehman <[email protected]> Reviewed-by: @jalehman
…nclaw#40126) Merged via squash. Prepared head SHA: 5228d19 Co-authored-by: jarimustonen <[email protected]> Co-authored-by: jalehman <[email protected]> Reviewed-by: @jalehman
…nclaw#40126) Merged via squash. Prepared head SHA: 5228d19 Co-authored-by: jarimustonen <[email protected]> Co-authored-by: jalehman <[email protected]> Reviewed-by: @jalehman
…nclaw#40126) Cherry-picked from fd2b3ed. Conflict resolution: Took upstream's buildMemoryPromptSection delegate pattern in system-prompt.ts. Kept our enhanced operator1 memory prompt in memory-core buildPromptSection with per-tool guidance. Kept our googlechat export and matrix/mattermost entrypoints in append-only files. Took upstream for plugin infrastructure (loader.ts, registry.ts, types.ts). (cherry picked from commit fd2b3ed)
Summary
system-prompt.ts, providing memory-core-specific instructions. When a third-party memory plugin is active (via the exclusive memory slot), these instructions are misleading.registerMemoryPromptSection(builder)to the plugin API. The active memory plugin registers its own system prompt builder callback. Moved the existing "Memory Recall" logic intomemory-coreas its registration.buildMemorySection()delegates to the registered callback.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
None
User-visible / Behavior Changes
None for default memory-core users. Memory plugins with
kind: "memory"can now register a custom system prompt section viaapi.registerMemoryPromptSection(builder).Security Impact (required)
Repro + Verification
Environment
Steps
api.registerMemoryPromptSection(builder)Expected
Actual (before fix)
Evidence
New unit tests in
prompt-section.test.ts: delegation, citationsMode passthrough, no-builder returns empty, last-registration-wins. All 43 existingsystem-prompt.test.tstests pass.Human Verification (required)
Compatibility / Migration
Failure Recovery (if this breaks)
Risks and Mitigations
None. Additive API change. Default behavior is identical — memory-core registers the same prompt content it previously hardcoded.