Repository navigation
fix(openclaw): support episodic memory type - #347
loveRhythm1990 merged 1 commit into
Conversation
OpenClaw omitted the core-supported episodic memory type from both runtime validation and the static plugin manifest. Add it to the shared type list and cover parser/manifest parity with regression tests. 由 LW 授权的 ChatGPT Business 智能体提交。
|
Code review: LGTM on The omission is real: Rust's canonical memory-type enum already includes Locally, all 205 OpenClaw tests and package build/verification passed. With the baseline config and manifest, the same config suite produced 2 failures and 19 passes. OpenClaw and Rust/MatrixOne CI passed, including the explicit restore/cleanup regressions. Coverage: 3/3 changed files reviewed (100%), 0 skipped; no actionable code issues found. There is a separate merge-queue configuration failure: Mergify reports that |
|
Non-blocking follow-up (medium severity, pre-existing): accepting The enum/manifest addition is correct. I additionally exercised the registered Using the actual registered This is an existing wiring defect, not a regression introduced by adding the enum member, so it can be handled separately. Please forward the selected types through both client and transport to the server's Review coverage remains 3/3 changed files, 0 skipped; the finding above is in unchanged surrounding code. |
aptend
left a comment
aptend
left a comment
There was a problem hiding this comment.
Independent deep review of exact head 167e014d58b8fe401431aad8525517e9e546f96f, base/merge-base 0695ad61eec9104478f05f5824b75bdbeeca5616. Read all three changed files, the historical review/discussion and surrounding config/tool/client/server consumers; no inline threads.
No blocking finding in this additive configuration/schema fix. The shared enum and checked-in manifest now agree with Rust's canonical six-type enum, including serialization/parse spelling episodic. The derived TypeScript type, runtime configuration validation, store/list tool schemas and validators, and initialized type counts consume the same shared list. No API/storage/auth/lifecycle change is introduced.
I independently confirmed the already-reported, unchanged retrieval-filter wiring gap: retrieve(memoryTypes) does not forward it, in both this head and base; the HTTP adapter also omits it. That is not caused or fixed by this enum addition. Approval covers accepting/advertising the valid type, not enforcement of episodic-only retrieval; no duplicate follow-up finding is being presented as a new regression.
Verified exact-head CI run 39507121400, job 118918724996, raw output on Node22.23.3: config suite21/21 and complete OpenClaw suite205/205 across12files PASS; bundle/package validation and all6 packed files PASS. git diff --check PASS. Reused qualified CI; no local npm install, database run, product edit or temporary probe claimed.
|
Follow-up implemented in #350: Validation on the exact published tree: baseline regression 1 failed / 5 passed; fixed focused suite 6/6, full OpenClaw suite 216/216, package validation and all 6 packed files passed, 由 LW 授权的 ChatGPT Business 智能体提交。 |
## What type of PR is this? - [x] fix (bug fix) ## Which issue(s) this PR fixes Follow-up to the pre-existing retrieval wiring defect documented during review of #347: #347 (comment) ## What this PR does / why we need it OpenClaw accepts `retrieveMemoryTypes` and `MemoriaClient.retrieve()` already exposes `memoryTypes`, but the selected values were dropped before the request reached the REST API. As a result, configured type filters could silently retrieve semantic/profile/etc. records together instead of restricting recall. This change: - forwards `memoryTypes` as `memory_types` from the client; - includes `memory_types` in the HTTP request body only when supplied; - preserves the existing omitted-filter request shape. The REST API already supports `RetrieveRequest.memory_types`. No server, storage, authentication, cache, or response parsing behavior changes. Scope: **3 files, +20/-0**. Base: `659a7779c1f3be787c4dba1d67cd12b998dd0b32`. Published commit: `ccb126290fcfbff1aefb323b0a1d40c2feb22fbb`. Tested/reviewed tree: `63a8d65e5bea6d830199cda6161fdce0fa7fcebc`. ## Reproduction and validation The identical regression test was run on current `main` and the fixed tree: - Current `main`: **1 failed / 5 passed**; the serialized body lacked `memory_types`. - Fixed focused suite: **6/6 passed**. - Full OpenClaw suite: **216/216 passed across 13 files**. - `npm run test:pack`: build/package validation passed; all **6 packed files** verified. - `git diff --check`: passed. Independent review agent: **APPROVE**, no blocking findings. It independently reran the focused suite, all 216 tests, package validation, and diff check against the exact tested tree. The remote commit above has that same tree. Remote Rust/MatrixOne and PR workflow checks are left to CI; no merge or deployment action is requested. 由 LW 授权的 ChatGPT Business 智能体提交。
## What type of PR is this? - [x] build / ci (build or CI changes) ## What this PR does / why we need it **Behavior change: this proposal restores valid configuration, but does not restore automatic enqueueing with the currently reported product settings. Maintainers would use `@mergifyio queue`. The enqueueing policy needs a maintainer decision before merge.** Remove rejected `queue_rules[].autoqueue` and migrate `commit_message_template` to `commit_message_format`. Keep one approval, no `do-not-merge`, and `base=main` as queue-level conditions. Squash merging, timeout, concurrency, and GitHub branch protection remain unchanged. Approver attribution becomes standard `Approved-by` trailers. Related: #285; configuration failures on #347, #348, and #350. ## Enqueueing decision - **Automatic:** enable Merge Protections in the dashboard, then explicitly configure a conditional list (not unconditional `true`): ```yaml merge_protections_settings: auto_merge_conditions: - "#approved-reviews-by>=1" - label!=do-not-merge - base=main ``` This is an alternative, not included in the patch. Once `autoqueue` is removed, do not rely on the bot to migrate that field automatically. - **Manual:** accept this proposal and use `@mergifyio queue` for eligible PRs. The live configuration check confirms Workflow Automation is disabled, so the retained `pull_request_rules` are inactive; this is now documented inline. The Merge Protections prerequisite comes from the bot's #285 notice; dashboard settings have not been independently verified or changed. ## Validation - Official schema validation, parsed-YAML gate checks, and `git diff --check` passed. - Live `Configuration changed` passed on `999ccb4`; the latest commit only adds comments and parses identically. Latest-head CI is pending. - No tracked references to the old approver-attribution syntax outside `.mergify.yml` were found. - `Mergify Merge Queue` still evaluates main's old configuration until merge, so its `autoqueue` error is separate from proposed-file validation. Only `.mergify.yml` changes. No merge or product-setting changes performed.
4 participants
由 LW 授权的 ChatGPT Business 智能体提交。
What type of PR is this?
What this PR fixes
Memoria core, REST and MCP support six memory types, including
episodic, but the OpenClaw plugin's canonicalMEMORIA_MEMORY_TYPESlist and static plugin manifest omitted it. As a result:retrieveMemoryTypes: ["episodic"]is rejected during plugin config parsing.episodicwhere a memory type is selected.Searches for OpenClaw/episodic,
MEMORIA_MEMORY_TYPES, andretrieveMemoryTypesfound no duplicate issue or fix PR.Change
Add
episodicto the shared OpenClaw memory-type enum and the checked-in plugin manifest, with regression coverage for both runtime parsing and manifest advertisement.Scope: 3 files, +21/-1, based on
main@0695ad61eec9104478f05f5824b75bdbeeca5616. No API, storage, auth, dependency, or workflow changes. The repository has nodevbranch, so this PR targetsmain.Verification
Node 24.19.0 / Vitest 4.1.11, from
plugins/openclaw:npm test: 205/205 passed across 12 files.npm run test:pack: passed (bundle build, package validation, 6 packed files).git diff --check: passed.Rust tests were not rerun locally because Cargo is unavailable. The patch is additive and directly matches the authoritative Rust core enum and existing MCP schemas, which already include
episodic. Upstream required CI remains authoritative.No merge, deployment, or package publication was performed.