feat: add OpenClaw client plugin#34
Merged
GustyCube merged 10 commits intoGustyCube:masterfrom Mar 12, 2026
Merged
Conversation
added 2 commits
March 6, 2026 11:10
Adds clients/openclaw/ — an OpenClaw plugin that bridges to Membrane for episodic memory. Features: - Event ingestion via after_agent_reply / after_tool_call hooks - membrane_search tool for episodic memory queries - Auto-context injection before agent turns - /membrane status command - Full TypeScript, zero custom gRPC (uses @gustycube/membrane) Uses the official @gustycube/membrane TypeScript client as dependency. 10 tests passing (vitest).
BREAKING: Replace module-level singletons with OpenClawMembranePlugin class. - Prevents gRPC connection leaks on re-activation - Enables proper unit testing without global state mutation - Add test/plugin.test.ts (11 tests for config, lifecycle, error paths) Total: 21 tests passing, zero TypeScript errors.
There was a problem hiding this comment.
Pull request overview
Adds a new clients/openclaw/ TypeScript plugin package that integrates OpenClaw hook events with Membrane using the existing @gustycube/membrane TS client, plus tests and documentation for installation and usage.
Changes:
- Introduces the OpenClaw→Membrane bridge plugin implementation (ingestion, retrieval, context injection, status).
- Adds mapping/config/types utilities and Vitest-based test coverage for mapping/config/plugin behavior.
- Adds packaging + manifest + README for distribution as
@vainplex/openclaw-membrane.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| clients/openclaw/src/index.ts | Core plugin class (activate/deactivate, event ingestion, search, context injection, status). |
| clients/openclaw/src/mapping.ts | Translates OpenClaw events/sensitivity into Membrane-oriented kinds, summaries, and tags. |
| clients/openclaw/src/types.ts | Plugin config + OpenClaw event/API types and defaults. |
| clients/openclaw/test/plugin.test.ts | Tests for config helpers and basic plugin behavior when disconnected. |
| clients/openclaw/test/mapping.test.ts | Tests for mapping helpers (kinds, summaries, tags, sensitivity). |
| clients/openclaw/package.json | Publishes the plugin package and declares dependency on @gustycube/membrane. |
| clients/openclaw/tsconfig.json | TypeScript build configuration for emitting dist/ declarations and ESM output. |
| clients/openclaw/vitest.config.ts | Vitest configuration for running the plugin tests. |
| clients/openclaw/openclaw.plugin.json | OpenClaw plugin manifest (hooks/tools/commands). |
| clients/openclaw/README.md | Installation, configuration, and usage documentation for the plugin. |
| clients/openclaw/.gitignore | Ignores local build artifacts for this subpackage. |
| .gitignore | Adds an ignore rule for clients/openclaw/package-lock.json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+90
to
+96
| } else { | ||
| await this.client.ingestEvent(event.hook, source, { | ||
| summary: summarize(event), | ||
| sensitivity, | ||
| tags, | ||
| }); | ||
| } |
There was a problem hiding this comment.
handleEvent routes all non-tool events through ingestEvent(...), even when mapEventKind returns "observation". This means observation hooks will be stored as event memories instead of observation memories. Consider adding a dedicated branch for kind === "observation" that calls client.ingestObservation(...) (or adjust mapEventKind/docs if observations are intentionally stored as events).
clients/openclaw/src/index.ts
Outdated
Comment on lines
+91
to
+94
| await this.client.ingestEvent(event.hook, source, { | ||
| summary: summarize(event), | ||
| sensitivity, | ||
| tags, |
There was a problem hiding this comment.
MembraneClient.ingestEvent(eventKind, ref, options) takes the second argument as ref, not source. Here source is being passed as ref, and options.source is never set, so the ingested event will have a default source and an unexpected ref value. Set options.source explicitly and pass an appropriate ref (e.g., sessionKey/message id) as the second parameter.
Suggested change
| await this.client.ingestEvent(event.hook, source, { | |
| summary: summarize(event), | |
| sensitivity, | |
| tags, | |
| const ref = event.hook; | |
| await this.client.ingestEvent(event.hook, ref, { | |
| summary: summarize(event), | |
| sensitivity, | |
| tags, | |
| source, |
clients/openclaw/src/index.ts
Outdated
Comment on lines
+105
to
+115
| options?: { limit?: number; memoryTypes?: string[]; minSalience?: number }, | ||
| ): Promise<MemoryRecord[]> { | ||
| if (!this.client) return []; | ||
|
|
||
| try { | ||
| const retrieveOpts: RetrieveOptions = { | ||
| limit: options?.limit ?? this.config.context_limit, | ||
| minSalience: options?.minSalience ?? this.config.min_salience, | ||
| }; | ||
| if (options?.memoryTypes) { | ||
| retrieveOpts.memoryTypes = options.memoryTypes as MemoryType[]; |
There was a problem hiding this comment.
search() accepts options.memoryTypes / options.minSalience (camelCase), but the OpenClaw tool manifest declares memory_types (snake_case) and no min_salience parameter. If OpenClaw passes tool args verbatim, these filters will be silently ignored. Consider supporting both naming styles (or aligning manifest/README + runtime parsing) to avoid confusing tool users.
Suggested change
| options?: { limit?: number; memoryTypes?: string[]; minSalience?: number }, | |
| ): Promise<MemoryRecord[]> { | |
| if (!this.client) return []; | |
| try { | |
| const retrieveOpts: RetrieveOptions = { | |
| limit: options?.limit ?? this.config.context_limit, | |
| minSalience: options?.minSalience ?? this.config.min_salience, | |
| }; | |
| if (options?.memoryTypes) { | |
| retrieveOpts.memoryTypes = options.memoryTypes as MemoryType[]; | |
| options?: { | |
| limit?: number; | |
| // camelCase (internal) and snake_case (tool manifest) naming variants | |
| memoryTypes?: string[]; | |
| memory_types?: string[]; | |
| minSalience?: number; | |
| min_salience?: number; | |
| }, | |
| ): Promise<MemoryRecord[]> { | |
| if (!this.client) return []; | |
| try { | |
| const effectiveMinSalience = | |
| options?.minSalience ?? options?.min_salience ?? this.config.min_salience; | |
| const effectiveMemoryTypes = options?.memoryTypes ?? options?.memory_types; | |
| const retrieveOpts: RetrieveOptions = { | |
| limit: options?.limit ?? this.config.context_limit, | |
| minSalience: effectiveMinSalience, | |
| }; | |
| if (effectiveMemoryTypes) { | |
| retrieveOpts.memoryTypes = effectiveMemoryTypes as MemoryType[]; |
clients/openclaw/src/types.ts
Outdated
Comment on lines
+10
to
+33
| /** Event buffer size for reliability */ | ||
| buffer_size: number; | ||
| /** Auto-inject context on agent start */ | ||
| auto_context: boolean; | ||
| /** Max memories to inject as context */ | ||
| context_limit: number; | ||
| /** Min salience for context injection */ | ||
| min_salience: number; | ||
| /** Memory types to include in context */ | ||
| context_types: string[]; | ||
| /** Flush interval in ms for buffered events */ | ||
| flush_interval_ms: number; | ||
| } | ||
|
|
||
| export const DEFAULT_CONFIG: PluginConfig = { | ||
| grpc_endpoint: "localhost:4222", | ||
| default_sensitivity: "low", | ||
| buffer_size: 100, | ||
| auto_context: true, | ||
| context_limit: 5, | ||
| min_salience: 0.3, | ||
| context_types: ["event", "tool_output", "observation"], | ||
| flush_interval_ms: 5000, | ||
| }; |
There was a problem hiding this comment.
PluginConfig defines buffer_size and flush_interval_ms (and README documents them), but the plugin implementation does not currently buffer events or perform periodic flushing. Either implement buffering using these settings or remove them from the config/defaults/docs to avoid a misleading configuration surface.
1. Observation routing: handleEvent now calls ingestObservation() for observation-kind events instead of misrouting through ingestEvent() 2. ingestEvent ref parameter: pass sessionKey (or hook name) as ref, set source explicitly in options — fixes wrong ref/source values 3. Snake_case support: search() now accepts both memory_types and memoryTypes, min_salience and minSalience — matches tool manifest 4. Remove dead config: buffer_size and flush_interval_ms removed from PluginConfig, DEFAULT_CONFIG, validateConfig, and README — the plugin does not implement buffering 22 tests passing.
Owner
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new OpenClaw Membrane plugin under Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as OpenClaw Agent
participant Plugin as OpenClawMembranePlugin
participant Membrane as Membrane gRPC Service
Agent->>Plugin: activate()
Plugin->>Membrane: connect(gRPC endpoint)
Membrane-->>Plugin: connection OK
Agent->>Plugin: handleEvent(event)
Plugin->>Plugin: map kind/sensitivity/tags
Plugin->>Membrane: ingest(memory record)
Membrane-->>Plugin: ack
Agent->>Plugin: search(query, options)
Plugin->>Membrane: query(filters)
Membrane-->>Plugin: results
Plugin-->>Agent: return results
Agent->>Plugin: getContext(agentId)
Plugin->>Membrane: fetch recent memories
Membrane-->>Plugin: records
Plugin-->>Agent: formatted context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
clients/openclaw/README.md (3)
80-96: Add language specifier for ASCII diagram.Consider using
```textfor the architecture diagram to satisfy markdown lint rules while preserving plain text rendering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/openclaw/README.md` around lines 80 - 96, The ASCII architecture diagram block starting with "OpenClaw Agent" should have a language specifier to satisfy markdown lint rules; update the fenced code block that contains the diagram (the lines showing "OpenClaw Agent" and the arrows to ingestEvent()/ingestToolOutput()/retrieve() and the Membrane (gRPC) box) from triple backticks to include a plain-text specifier (e.g., ```text) so the diagram still renders as plain text while linting cleanly.
61-63: Add language specifier to fenced code blocks.Per static analysis (markdownlint MD040), fenced code blocks should have a language specified for better rendering and accessibility. This block could use
javascriptorts.-``` -membrane_search("what happened in yesterday's meeting", { limit: 10 }) -``` +```javascript +membrane_search("what happened in yesterday's meeting", { limit: 10 }) +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/openclaw/README.md` around lines 61 - 63, The fenced code block showing the example call to membrane_search lacks a language tag; update the block around membrane_search("what happened in yesterday's meeting", { limit: 10 }) to include a language specifier (e.g., ```javascript or ```ts) so markdownlint MD040 is satisfied and syntax highlighting is enabled.
73-76: Add language specifier for command output.Consider adding a language tag (e.g.,
textorconsole) to this code block for consistency with markdown lint rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/openclaw/README.md` around lines 73 - 76, The code block showing the command output (the block containing "/membrane → Membrane: connected (localhost:4222) | 1,247 records | 3 memory types") lacks a language tag; update that fenced block to include a language specifier such as "text" or "console" (e.g., change the opening ``` to ```text) so it complies with markdown lint rules and renders consistently..gitignore (1)
68-68: Consider committingpackage-lock.jsonfor reproducible builds.Ignoring
package-lock.jsonmeans dependency versions won't be locked across environments, which can lead to inconsistent builds. The npm recommendation is to commit lock files for applications and packages.If this is intentional (e.g., to avoid monorepo conflicts or because the package is treated as a library), consider documenting the reasoning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 68, The .gitignore currently excludes clients/openclaw/package-lock.json which prevents committing the lockfile; either remove the clients/openclaw/package-lock.json entry so the package-lock.json for the openclaw package is tracked (ensuring reproducible installs), or if excluding it is intentional, add a short comment in the .gitignore explaining why (e.g., monorepo lockfile strategy or library policy) so reviewers understand the rationale.clients/openclaw/package.json (1)
31-31: Build script is not cross-platform.The
rm -rfcommand won't work on Windows. Consider using a cross-platform alternative.♻️ Cross-platform alternative
Option 1: Use
rimraf(common approach):+"rimraf": "^5.0.0"-"build": "rm -rf dist && tsc", +"build": "rimraf dist && tsc",Option 2: Use TypeScript's
--cleanbehavior by relying ontscto overwrite, or use npm scripts with Node.js:-"build": "rm -rf dist && tsc", +"build": "node -e \"require('fs').rmSync('dist',{recursive:true,force:true})\" && tsc",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/openclaw/package.json` at line 31, The build script currently uses a POSIX-only command ("rm -rf dist && tsc") which fails on Windows; update the "build" script in package.json (the "build" npm script) to use a cross-platform removal approach such as adding and using the rimraf package (add rimraf as a devDependency and replace the removal step with rimraf dist) or switch to a TypeScript build command that doesn't require manual deletion (e.g., use tsc --build or another npm/Node-based deletion method) so the script works across platforms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Around line 14-19: Update the "parameters" entry so it conforms to JSON
Schema: make it an object with "type": "object", move the existing keys under a
"properties" map (keeping descriptions and types for "query", "limit",
"memory_types", "min_salience"), and add a "required" array that includes
"query" so the membrane_search tool enforces the query parameter; reference the
existing "parameters" object and the "query" property when making the change.
In `@clients/openclaw/src/index.ts`:
- Around line 97-99: The event ref generation uses a non-unique value (const ref
= event.sessionKey ?? event.hook) causing collisions; change the ref calculation
used in the call to this.client.ingestEvent(...) to produce a unique identifier
(e.g., combine sessionKey or hook with a high-resolution timestamp and/or a
random UUID/nonce). Locate the ref assignment in clients/openclaw/src/index.ts
(the const ref and the this.client.ingestEvent(event.hook, ref, {...}) call) and
replace it so each ingested event gets a truly unique ref (for example append
Date.now() and/or crypto.randomUUID() or a short random string to the existing
key).
- Around line 22-33: The validateConfig function currently lets through
NaN/negative/out-of-range values and empty arrays which then override
DEFAULT_CONFIG in createConfig; update validateConfig to reject invalid scalars
(e.g., ensure context_limit is a finite positive integer, min_salience is a
finite number within the allowed range [0..1] or configured bounds, and not
NaN), validate default_sensitivity is a non-empty/allowed string if there is a
closed set of sensitivities, and for context_types only set result.context_types
when Array.isArray and the filtered array has length > 0 (otherwise leave
undefined so DEFAULT_CONFIG wins); keep auto_context and grpc_endpoint checks
but ensure no invalid values are assigned so DEFAULT_CONFIG remains when inputs
are invalid.
- Around line 172-176: The current catch block treats any failure from
this.client.getMetrics() as a disconnected client, causing false negatives;
change the logic in the method using this.client.getMetrics() so that a metrics
error does not flip connected to false—call getMetrics() in a try/catch and on
error return { connected: true, endpoint: this.config.grpc_endpoint, metrics:
undefined } (or include an error field) unless you detect an explicit
transport/connection error (e.g., client is null/undefined or the error
message/status indicates refusal); reference this.client, getMetrics(), and
this.config.grpc_endpoint when locating and updating the code.
- Around line 156-159: The current mapping of records only outputs r.id; update
the mapping in the records -> lines creation to inject the human-readable memory
summary from the record payload for episodic memories (use
r.payload.timeline[i].summary) and fall back safely if missing (e.g.,
r.payload.summary or r.payload). In the function that builds the lines array
(the records.map using MemoryRecord), replace the `${r.id}` part with the
extracted summary value and add defensive checks for undefined
payload/timeline/summary so the code doesn't throw if those fields are absent.
---
Nitpick comments:
In @.gitignore:
- Line 68: The .gitignore currently excludes clients/openclaw/package-lock.json
which prevents committing the lockfile; either remove the
clients/openclaw/package-lock.json entry so the package-lock.json for the
openclaw package is tracked (ensuring reproducible installs), or if excluding it
is intentional, add a short comment in the .gitignore explaining why (e.g.,
monorepo lockfile strategy or library policy) so reviewers understand the
rationale.
In `@clients/openclaw/package.json`:
- Line 31: The build script currently uses a POSIX-only command ("rm -rf dist &&
tsc") which fails on Windows; update the "build" script in package.json (the
"build" npm script) to use a cross-platform removal approach such as adding and
using the rimraf package (add rimraf as a devDependency and replace the removal
step with rimraf dist) or switch to a TypeScript build command that doesn't
require manual deletion (e.g., use tsc --build or another npm/Node-based
deletion method) so the script works across platforms.
In `@clients/openclaw/README.md`:
- Around line 80-96: The ASCII architecture diagram block starting with
"OpenClaw Agent" should have a language specifier to satisfy markdown lint
rules; update the fenced code block that contains the diagram (the lines showing
"OpenClaw Agent" and the arrows to ingestEvent()/ingestToolOutput()/retrieve()
and the Membrane (gRPC) box) from triple backticks to include a plain-text
specifier (e.g., ```text) so the diagram still renders as plain text while
linting cleanly.
- Around line 61-63: The fenced code block showing the example call to
membrane_search lacks a language tag; update the block around
membrane_search("what happened in yesterday's meeting", { limit: 10 }) to
include a language specifier (e.g., ```javascript or ```ts) so markdownlint
MD040 is satisfied and syntax highlighting is enabled.
- Around line 73-76: The code block showing the command output (the block
containing "/membrane → Membrane: connected (localhost:4222) | 1,247 records | 3
memory types") lacks a language tag; update that fenced block to include a
language specifier such as "text" or "console" (e.g., change the opening ``` to
```text) so it complies with markdown lint rules and renders consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a14e7e0d-a9e9-4577-8968-fdb262beba94
📒 Files selected for processing (12)
.gitignoreclients/openclaw/.gitignoreclients/openclaw/README.mdclients/openclaw/openclaw.plugin.jsonclients/openclaw/package.jsonclients/openclaw/src/index.tsclients/openclaw/src/mapping.tsclients/openclaw/src/types.tsclients/openclaw/test/mapping.test.tsclients/openclaw/test/plugin.test.tsclients/openclaw/tsconfig.jsonclients/openclaw/vitest.config.ts
Owner
|
@alberthild There are a few more issues here that may need to be resolved, thanks |
- Restore .coderabbit.yaml (accidentally deleted) - Restore --version flag in cmd/membraned/main.go (accidentally reverted) - Fix openclaw.plugin.json to use proper JSON Schema with required - Harden validateConfig against NaN, negative, OOB values - Make event ref unique (timestamp-based) to avoid collisions - Show payload summary in context injection instead of just record ID - Fix getStatus() false disconnect on metrics failure - Add language specifiers to README code blocks - Add validation edge case tests (26/26 pass)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
clients/openclaw/src/index.ts (1)
165-169:⚠️ Potential issue | 🟠 MajorRead episodic summaries from
payload.timeline, not just top-level payload fields.This still misses the normal Membrane episodic shape, where the readable text lives on
payload.timeline[].summary. For typical episodic hits, the current fallback will keep collapsing tor.id, so the injected context remains opaque to the model. (pkg.go.dev)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/openclaw/src/index.ts` around lines 165 - 169, The mapping that builds human-readable summaries (inside the records.map callback for MemoryRecord) only checks payload.summary and payload.content; update it to inspect payload.timeline (an array) and extract the first non-empty timeline entry summary (payload.timeline[i].summary) before falling back to payload.content or r.id. In the records.map callback, safely cast/check payload.timeline as Array<Record<string, unknown>>, iterate or use find to pick a timeline entry with a summary string, use that as the summary variable, and keep existing fallbacks to payload.content and r.id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Around line 1-32: The manifest is missing required top-level fields `id` and
`configSchema` which prevents OpenClaw from discovering and validating the
plugin; add an `id` field (a canonical identifier for this plugin associated
with the existing "name": "openclaw-membrane") and a `configSchema` top-level
field containing the plugin configuration JSON Schema (if there are no runtime
settings, use the empty schema { "type": "object", "additionalProperties": false
}); ensure these fields are placed alongside the existing fields (e.g., near
"name" and "version") so OpenClaw can load and validate the plugin.
In `@clients/openclaw/README.md`:
- Around line 30-52: Update the Configuration section in
clients/openclaw/README.md to show the correct OpenClaw plugin config location
and structure: replace the incorrect path
`~/.openclaw/plugins/openclaw-membrane/config.json` and flat JSON with the
nested `plugins.entries.<id>.config` format (e.g.,
`plugins.entries.openclaw-membrane.config`) so the plugin manifest's
`configSchema` will be respected; use the example structure where the plugin
entry includes `enabled` and a `config` object containing `grpc_endpoint`,
`default_sensitivity`, `auto_context`, `context_limit`, `min_salience`, and
`context_types`, and update the prose and example JSON accordingly.
In `@clients/openclaw/src/index.ts`:
- Around line 34-36: The code is accepting arbitrary strings for
context_types/memory types; update validateConfig to filter raw.context_types
against Membrane's supported MemoryType values
('episodic','working','semantic','competence','plan_graph') before assigning to
result.context_types, and change DEFAULT_CONFIG to use only valid memory types
instead of 'event','tool_output','observation'; likewise in the search method
validate and filter incoming memoryTypes/memory_types params against the same
allowed set before casting, and in getContext validate this.config.context_types
(filter out invalid entries) before casting to MemoryType[] so invalid values
are never sent to the Membrane server (referencing validateConfig,
DEFAULT_CONFIG, search, getContext, memoryTypes/memory_types, and
this.config.context_types).
In `@clients/openclaw/test/plugin.test.ts`:
- Around line 38-71: Add regression tests that assert unsupported memoryTypes
are rejected by validateConfig (call validateConfig with a mix like {
memory_types: ["episodic", "unsupported", 123] } and expect only the supported
string entries or undefined per current contract) and add tests for getContext
to ensure retrieval payloads are converted into episodic summaries (mock
retrieval results and call getContext to assert the returned context includes
the expected episodic summary strings/fields). Target the validateConfig and
getContext behaviors and reference those symbols when adding the new test cases
so the retrieval filters and context formatting regressions are covered.
---
Duplicate comments:
In `@clients/openclaw/src/index.ts`:
- Around line 165-169: The mapping that builds human-readable summaries (inside
the records.map callback for MemoryRecord) only checks payload.summary and
payload.content; update it to inspect payload.timeline (an array) and extract
the first non-empty timeline entry summary (payload.timeline[i].summary) before
falling back to payload.content or r.id. In the records.map callback, safely
cast/check payload.timeline as Array<Record<string, unknown>>, iterate or use
find to pick a timeline entry with a summary string, use that as the summary
variable, and keep existing fallbacks to payload.content and r.id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a11d51b8-0987-4019-9f09-57fc28e1cc96
📒 Files selected for processing (6)
.coderabbit.yamlclients/openclaw/README.mdclients/openclaw/openclaw.plugin.jsonclients/openclaw/src/index.tsclients/openclaw/test/plugin.test.tscmd/membraned/main.go
Comment on lines
+38
to
+71
| describe("validateConfig", () => { | ||
| it("returns empty for undefined input", () => { | ||
| expect(validateConfig(undefined)).toEqual({}); | ||
| }); | ||
|
|
||
| it("filters context_types to strings only", () => { | ||
| const result = validateConfig({ context_types: ["event", 42, "observation"] }); | ||
| expect(result.context_types).toEqual(["event", "observation"]); | ||
| }); | ||
|
|
||
| it("rejects NaN and negative context_limit", () => { | ||
| expect(validateConfig({ context_limit: NaN })).toEqual({}); | ||
| expect(validateConfig({ context_limit: -1 })).toEqual({}); | ||
| expect(validateConfig({ context_limit: 0 })).toEqual({}); | ||
| expect(validateConfig({ context_limit: 3.5 })).toEqual({}); | ||
| }); | ||
|
|
||
| it("rejects out-of-range min_salience", () => { | ||
| expect(validateConfig({ min_salience: -0.1 })).toEqual({}); | ||
| expect(validateConfig({ min_salience: 1.5 })).toEqual({}); | ||
| expect(validateConfig({ min_salience: NaN })).toEqual({}); | ||
| }); | ||
|
|
||
| it("accepts valid min_salience", () => { | ||
| expect(validateConfig({ min_salience: 0 })).toEqual({ min_salience: 0 }); | ||
| expect(validateConfig({ min_salience: 0.5 })).toEqual({ min_salience: 0.5 }); | ||
| expect(validateConfig({ min_salience: 1 })).toEqual({ min_salience: 1 }); | ||
| }); | ||
|
|
||
| it("drops empty context_types array to preserve defaults", () => { | ||
| const result = validateConfig({ context_types: [42, true] }); | ||
| expect(result.context_types).toBeUndefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add regression coverage for retrieval filters and context formatting.
The suite never asserts that unsupported memoryTypes are rejected or that getContext() renders episodic summaries from retrieved payloads. Those are part of the plugin’s public contract and would have caught the current retrieval/context bugs.
Also applies to: 93-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/openclaw/test/plugin.test.ts` around lines 38 - 71, Add regression
tests that assert unsupported memoryTypes are rejected by validateConfig (call
validateConfig with a mix like { memory_types: ["episodic", "unsupported", 123]
} and expect only the supported string entries or undefined per current
contract) and add tests for getContext to ensure retrieval payloads are
converted into episodic summaries (mock retrieval results and call getContext to
assert the returned context includes the expected episodic summary
strings/fields). Target the validateConfig and getContext behaviors and
reference those symbols when adding the new test cases so the retrieval filters
and context formatting regressions are covered.
- Add id and configSchema to openclaw.plugin.json (OpenClaw discovery) - Fix DEFAULT_CONFIG to use valid Membrane MemoryTypes (episodic/semantic/competence) instead of invalid types (event/tool_output/observation) - Filter context_types against VALID_MEMORY_TYPES in validateConfig and search() - Extract episodic timeline summaries in context injection (payload.timeline[].summary) - Update README config path to OpenClaw plugins.entries format - Add regression tests for MemoryType validation (28/28 pass)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Around line 31-37: The manifest's memory_types parameter currently documents
incorrect values; update the JSON schema for the "memory_types" property in
openclaw.plugin.json to use an array of strings with an enum of the
runtime-accepted values ["episodic", "working", "semantic", "competence",
"plan_graph"] and update the description to match those allowed values so
callers sending documented filters are not dropped; ensure the schema still
declares "type": "array" and each item as "type": "string" with the enum above
for the "memory_types" property.
In `@clients/openclaw/src/index.ts`:
- Around line 136-150: The search implementation must reject invalid per-call
overrides instead of widening the query: in the search() path, validate options
before building retrieveOpts—if options.memoryTypes (effectiveMemoryTypes) is
provided, filter against VALID_MEMORY_TYPES and if validTypes.length === 0 throw
an error (do not omit the filter); also validate options.limit and
options.minSalience (ensure they are numbers and within acceptable ranges /
non-negative) before assigning to retrieveOpts.limit and
retrieveOpts.minSalience; update the logic around effectiveMemoryTypes,
retrieveOpts, VALID_MEMORY_TYPES, and any callers of search() so invalid
overrides are rejected rather than silently ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a1522bf8-f07a-4bdc-90f4-8b621627131b
📒 Files selected for processing (5)
clients/openclaw/README.mdclients/openclaw/openclaw.plugin.jsonclients/openclaw/src/index.tsclients/openclaw/src/types.tsclients/openclaw/test/plugin.test.ts
Comment on lines
+136
to
+150
| const effectiveMemoryTypes = options?.memoryTypes ?? options?.memory_types; | ||
| const effectiveMinSalience = | ||
| options?.minSalience ?? options?.min_salience ?? this.config.min_salience; | ||
|
|
||
| const retrieveOpts: RetrieveOptions = { | ||
| limit: options?.limit ?? this.config.context_limit, | ||
| minSalience: effectiveMinSalience, | ||
| }; | ||
| if (effectiveMemoryTypes) { | ||
| const validTypes = effectiveMemoryTypes.filter( | ||
| (t) => (VALID_MEMORY_TYPES as readonly string[]).includes(t), | ||
| ); | ||
| if (validTypes.length > 0) { | ||
| retrieveOpts.memoryTypes = validTypes as MemoryType[]; | ||
| } |
There was a problem hiding this comment.
Reject invalid per-call overrides instead of widening the search.
validateConfig() protects the config path, but search() still forwards raw limit/minSalience overrides and drops an explicit memoryTypes filter entirely when every requested type is invalid. That turns a bad membrane_search call into a semantically different query against all memory types.
Proposed fix
- const retrieveOpts: RetrieveOptions = {
- limit: options?.limit ?? this.config.context_limit,
- minSalience: effectiveMinSalience,
- };
+ const retrieveOpts: RetrieveOptions = {
+ limit: this.config.context_limit,
+ minSalience: this.config.min_salience,
+ };
+ if (typeof options?.limit === "number" && Number.isInteger(options.limit) && options.limit > 0) {
+ retrieveOpts.limit = options.limit;
+ }
+ if (
+ typeof effectiveMinSalience === "number" &&
+ Number.isFinite(effectiveMinSalience) &&
+ effectiveMinSalience >= 0 &&
+ effectiveMinSalience <= 1
+ ) {
+ retrieveOpts.minSalience = effectiveMinSalience;
+ }
if (effectiveMemoryTypes) {
const validTypes = effectiveMemoryTypes.filter(
(t) => (VALID_MEMORY_TYPES as readonly string[]).includes(t),
);
- if (validTypes.length > 0) {
- retrieveOpts.memoryTypes = validTypes as MemoryType[];
- }
+ if (validTypes.length === 0) return [];
+ retrieveOpts.memoryTypes = validTypes as MemoryType[];
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/openclaw/src/index.ts` around lines 136 - 150, The search
implementation must reject invalid per-call overrides instead of widening the
query: in the search() path, validate options before building retrieveOpts—if
options.memoryTypes (effectiveMemoryTypes) is provided, filter against
VALID_MEMORY_TYPES and if validTypes.length === 0 throw an error (do not omit
the filter); also validate options.limit and options.minSalience (ensure they
are numbers and within acceptable ranges / non-negative) before assigning to
retrieveOpts.limit and retrieveOpts.minSalience; update the logic around
effectiveMemoryTypes, retrieveOpts, VALID_MEMORY_TYPES, and any callers of
search() so invalid overrides are rejected rather than silently ignored.
- Update memory_types in plugin.json with items enum (episodic/working/semantic/competence/plan_graph) - Return early with warning when all provided memoryTypes are invalid in search()
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Line 35: The `limit` parameter in the plugin schema is declared as "type":
"number" but should be an integer to match `context_limit` and prevent
fractional values; update the `limit` property's schema to use "type": "integer"
(and keep the description/default intact) so validation and consumer behavior
are consistent with `context_limit`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1e5dd51-d6f7-4375-aac6-7d6c591e0a66
📒 Files selected for processing (2)
clients/openclaw/openclaw.plugin.jsonclients/openclaw/src/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Around line 31-40: The tool schema currently allows invalid values for search
parameters; update the JSON schema under "parameters" -> "properties" to enforce
stricter bounds: add "minimum": 1 (and optionally "default": 5) to the "limit"
property to prevent zero/negative results, and add "minimum": 0 and "maximum": 1
(and optionally "default": 0.3) to the "min_salience" property to constrain it
to [0,1]; keep the existing "memory_types" enum and required "query" unchanged
so LLM calls are rejected early when out-of-range values are supplied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4304d188-24d1-4c3a-ba00-3ea763c435db
📒 Files selected for processing (1)
clients/openclaw/openclaw.plugin.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Around line 2-6: The manifest for the OpenClaw plugin (id "openclaw-membrane",
name "openclaw-membrane", version "0.4.0") is missing the top-level "kind"
field; add "kind": "memory" at the top level of openclaw.plugin.json so the
platform treats this extension as a memory plugin and includes it in
plugins.slots.memory selection/exclusivity logic, keeping the rest of the
configSchema and fields unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c7ee643-9e87-4767-910f-326db479d5cb
📒 Files selected for processing (1)
clients/openclaw/openclaw.plugin.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/openclaw/openclaw.plugin.json`:
- Around line 32-40: The tool parameter schema for membrane_search is currently
permissive and allows unknown keys; update the parameters object for the
membrane_search function in openclaw.plugin.json to include
"additionalProperties": false (matching the pattern used by configSchema) so
extra/hallucinated arguments are rejected by the JSON-schema validation before
reaching the handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4a9bdb5-5079-42e1-aaa1-9f578748bacd
📒 Files selected for processing (1)
clients/openclaw/openclaw.plugin.json
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
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.
Closes #1
Hey! Sorry for the delay — here's the OpenClaw client plugin as promised.
What's in this PR
clients/openclaw/— a TypeScript OpenClaw plugin that bridges to Membrane for episodic memory.Features
after_agent_reply/after_tool_callhooksmembrane_searchtool for episodic memory queries/membranestatus commandArchitecture
Uses
@gustycube/membrane(your official TS client) as the only dependency — zero custom gRPC code. The plugin maps OpenClaw events to Membrane ingestion calls and exposes retrieval as a tool.Files
src/index.ts— plugin lifecycle, hooks, search, context injectionsrc/mapping.ts— OpenClaw event → Membrane format mappingsrc/types.ts— config typestest/mapping.test.ts— 10 tests (vitest)openclaw.plugin.json— plugin manifestREADME.md— install, config, usage docsTesting
Happy to adjust anything — let me know!
Summary by CodeRabbit
New Features
Documentation
Tests
Chores