Conversation
Member
Author
|
bugbot run |
📝 WalkthroughWalkthroughUpdated version metadata and integrated two plugin improvements: Hermes Agent's installer now robustly detects and configures the memory provider block with improved idempotency, and OpenClaw plugin now tolerates a host-owned Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 472d840. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
nowledge-mem-hermes/tests/test_setup.sh (1)
36-46: Good coverage of key scenarios, but missing test foralreadycase.The tests cover empty-provider, missing-provider, and conflict cases. Consider adding a test for the idempotent re-run case where
provider: "nowledge-mem"already exists:💡 Suggested additional test case
run_with_config \ "already-set" \ $'memory:\n provider: "nowledge-mem"' \ 'provider: "nowledge-mem"' \ '[ok] memory.provider already set'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-hermes/tests/test_setup.sh` around lines 36 - 46, Add an idempotent test case to cover when memory.provider is already set: add a call to the existing helper run_with_config named "already-set" that passes input YAML with memory:\n provider: "nowledge-mem" and expects the output 'provider: "nowledge-mem"' and the success message '[ok] memory.provider already set' so the test suite verifies re-running the config is a no-op; place it alongside the existing run_with_config tests for "empty-provider" and "missing-provider".nowledge-mem-hermes/setup.sh (1)
62-64: Consider edge case:memory:with inline value.The regex
^memory:\s*(#.*)?$only matchesmemory:with optional trailing comment, notmemory: {...}ormemory: some_value. If a user has an inline YAML value likememory: null, this would returnmissing-memoryinstead of detecting the block.This is likely acceptable since Hermes expects a mapping under
memory:, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-hermes/setup.sh` around lines 62 - 64, The current regex ^memory:\s*(#.*)?$ only matches a bare `memory:` with optional comment; update the matching logic so the `memory:` line is detected even when it has an inline value (e.g. `memory: null`, `memory: { ... }`, or `memory: some_value`) by changing the pattern to allow non-newline content after the colon or by separately testing for a line that starts with `memory:` and then checking whether the remainder is empty or an inline value; set memory_idx when such a line is found and, if the inline value is present, handle it explicitly (e.g., treat as missing-block error or convert/validate the inline value) so downstream logic that uses memory_idx behaves correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nowledge-mem-hermes/CHANGELOG.md`:
- Around line 7-12: The CHANGELOG.md currently has two consecutive "### Fixed"
headings causing ambiguity; edit CHANGELOG.md to either remove the duplicate
second "### Fixed" heading (if it was accidentally duplicated) or move the
orphaned fixes under the correct version header (e.g., insert the appropriate
version line above the second "### Fixed" or merge its items into the existing
0.5.8 "### Fixed" block); ensure the final file has a single "### Fixed" section
per version and that all fix bullets remain under the intended version heading.
In `@nowledge-mem-openclaw-plugin/README.md`:
- Line 243: Reword the ambiguous phrase around the top-level `dreaming` object
to clearly state when the note applies: specify that this applies when OpenClaw
is the owner of the memory slot and the Nowledge Mem OpenClaw plugin is attached
while `memory-core` is the primary memory handler; update the README sentence
that mentions `dreaming` so it reads something like “If OpenClaw (as the
memory-slot owner) stores a top-level `dreaming` object on this plugin while the
Nowledge Mem OpenClaw plugin is attached and `memory-core` is primary, that is
expected,” and ensure related docs, validator scenarios, and plugin hook
semantics references to `dreaming`, `memory-core`, and the Nowledge Mem OpenClaw
plugin are updated for consistency.
---
Nitpick comments:
In `@nowledge-mem-hermes/setup.sh`:
- Around line 62-64: The current regex ^memory:\s*(#.*)?$ only matches a bare
`memory:` with optional comment; update the matching logic so the `memory:` line
is detected even when it has an inline value (e.g. `memory: null`, `memory: {
... }`, or `memory: some_value`) by changing the pattern to allow non-newline
content after the colon or by separately testing for a line that starts with
`memory:` and then checking whether the remainder is empty or an inline value;
set memory_idx when such a line is found and, if the inline value is present,
handle it explicitly (e.g., treat as missing-block error or convert/validate the
inline value) so downstream logic that uses memory_idx behaves correctly.
In `@nowledge-mem-hermes/tests/test_setup.sh`:
- Around line 36-46: Add an idempotent test case to cover when memory.provider
is already set: add a call to the existing helper run_with_config named
"already-set" that passes input YAML with memory:\n provider: "nowledge-mem"
and expects the output 'provider: "nowledge-mem"' and the success message '[ok]
memory.provider already set' so the test suite verifies re-running the config is
a no-op; place it alongside the existing run_with_config tests for
"empty-provider" and "missing-provider".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 559dccd7-e1c4-4364-97a4-6f9c6c91f9d5
⛔ Files ignored due to path filters (1)
nowledge-mem-openclaw-plugin/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
integrations.jsonnowledge-mem-hermes/CHANGELOG.mdnowledge-mem-hermes/README.mdnowledge-mem-hermes/plugin.yamlnowledge-mem-hermes/setup.shnowledge-mem-hermes/tests/test_setup.shnowledge-mem-openclaw-plugin/CHANGELOG.mdnowledge-mem-openclaw-plugin/README.mdnowledge-mem-openclaw-plugin/openclaw.plugin.jsonnowledge-mem-openclaw-plugin/package.jsonnowledge-mem-openclaw-plugin/scripts/validate-plugin.mjsnowledge-mem-openclaw-plugin/src/config.jsnowledge-mem-openclaw-plugin/tests/space-config.test.mjs
Comment on lines
+7
to
12
| ### Fixed | ||
|
|
||
| - `setup.sh` now repairs an existing `memory:` block when `provider` is missing or set to an empty string. Earlier builds installed the plugin files correctly but still exited with code `1`, which left users with a half-successful setup until they edited `config.yaml` by hand. | ||
| - The installer still refuses to overwrite a different non-empty `memory.provider` silently. It now explains that conflict directly instead of treating every non-ready `memory:` block as the same generic failure. | ||
|
|
||
| ### Fixed |
There was a problem hiding this comment.
Duplicate ### Fixed section creates ambiguous changelog structure.
There are two consecutive ### Fixed headings. The second one (line 12) appears to belong to a previous version but is now orphaned under 0.5.8. This should likely be restructured:
♻️ Suggested fix
## [0.5.8] - 2026-04-14
### Fixed
- `setup.sh` now repairs an existing `memory:` block when `provider` is missing or set to an empty string. Earlier builds installed the plugin files correctly but still exited with code `1`, which left users with a half-successful setup until they edited `config.yaml` by hand.
- The installer still refuses to overwrite a different non-empty `memory.provider` silently. It now explains that conflict directly instead of treating every non-ready `memory:` block as the same generic failure.
+- Invalid `timeout` values in `~/.hermes/nowledge-mem.json` no longer crash the provider. Hermes now falls back to the default timeout and keeps the provider available.
+- An explicit empty `space` in Hermes config now stays on `Default` instead of falling through to ambient `NMEM_SPACE`.
+- ...remaining fixes from line 14-19...
-### Fixed
-
-- Invalid `timeout` values in `~/.hermes/nowledge-mem.json` no longer crash the provider. Hermes now falls back to the default timeout and keeps the provider available.
-...Alternatively, if those fixes belong to a different version, add the appropriate version header above line 12.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-hermes/CHANGELOG.md` around lines 7 - 12, The CHANGELOG.md
currently has two consecutive "### Fixed" headings causing ambiguity; edit
CHANGELOG.md to either remove the duplicate second "### Fixed" heading (if it
was accidentally duplicated) or move the orphaned fixes under the correct
version header (e.g., insert the appropriate version line above the second "###
Fixed" or merge its items into the existing 0.5.8 "### Fixed" block); ensure the
final file has a single "### Fixed" section per version and that all fix bullets
remain under the intended version heading.
|
|
||
| Requires OpenClaw >= 2026.4.5. | ||
|
|
||
| If OpenClaw stores a top-level `dreaming` object on this plugin while it owns the memory slot, that is expected. Dreaming remains OpenClaw-native host config; this plugin accepts it so memory-core can run the dreaming engine alongside Nowledge Mem without registration errors. |
There was a problem hiding this comment.
Clarify slot-ownership wording to avoid configuration ambiguity.
Line 243 says “while it owns the memory slot” inside a section that explains corpus supplement when memory-core is primary; that can confuse operators about when this note applies.
✏️ Suggested wording
-If OpenClaw stores a top-level `dreaming` object on this plugin while it owns the memory slot, that is expected. Dreaming remains OpenClaw-native host config; this plugin accepts it so memory-core can run the dreaming engine alongside Nowledge Mem without registration errors.
+OpenClaw may persist a top-level `dreaming` object in this plugin config. That is host-owned config: this plugin only tolerates/preserves it and does not execute dreaming itself, preventing registration errors.Based on learnings: Update design documentation, validator scenarios, and plugin implementation together when OpenClaw plugin hook semantics change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-openclaw-plugin/README.md` at line 243, Reword the ambiguous
phrase around the top-level `dreaming` object to clearly state when the note
applies: specify that this applies when OpenClaw is the owner of the memory slot
and the Nowledge Mem OpenClaw plugin is attached while `memory-core` is the
primary memory handler; update the README sentence that mentions `dreaming` so
it reads something like “If OpenClaw (as the memory-slot owner) stores a
top-level `dreaming` object on this plugin while the Nowledge Mem OpenClaw
plugin is attached and `memory-core` is primary, that is expected,” and ensure
related docs, validator scenarios, and plugin hook semantics references to
`dreaming`, `memory-core`, and the Nowledge Mem OpenClaw plugin are updated for
consistency.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Medium Risk
Moderate risk because
nowledge-mem-hermes/setup.shnow programmatically edits users’config.yamlvia a custom Python/YAML-ish parser, which could mis-handle unusual formatting; OpenClaw changes are mostly additive schema/validation tweaks.Overview
OpenClaw plugin: bumps to
0.8.14and updates config handling to accept a top-level host-owneddreamingobject (treated as pass-through) so plugin registration doesn’t fail when OpenClaw stores dreaming settings on the memory-slot owner. This is reflected across the manifest/schema (openclaw.plugin.json), validator (validate-plugin.mjsdefault-check logic for keys without defaults), config parser allowlist (src/config.js), and a new test covering thedreamingkey.Hermes provider: bumps to
0.5.8and fixessetup.shso it’s truly idempotent: when~/.hermes/config.yamlalready containsmemory:it now inserts/fillsmemory.providerif missing/empty, but fails with a clear message if a different non-empty provider is configured. Adds a small shell regression test (tests/test_setup.sh) and updates README/changelog accordingly.Reviewed by Cursor Bugbot for commit 472d840. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Updates