feat: OpenClaw engram plugin — persistent memory for AI agents#5
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughДобавлен новый плагин OpenClaw Engram: REST‑клиент с трекером доступности, форматирование/инъекция контекста, набор хуков, CLI и инструменты для памяти; также обновлена обработка Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as rgba(30,144,255,0.5) OpenClaw Plugin
participant Client as rgba(34,139,34,0.5) EngramRestClient
participant Server as rgba(178,34,34,0.5) Engram API
participant Formatter as rgba(218,165,32,0.5) ContextFormatter
participant Session as rgba(128,0,128,0.5) OpenClaw Session
Plugin->>Client: GET /context/inject?agent_id=...
Client->>Server: HTTP GET /context/inject
Server-->>Client: observations JSON
Client-->>Plugin: ContextInjectResponse
Plugin->>Formatter: formatContext(observations, tokenBudget)
Formatter-->>Plugin: XML context + injectedIds
Plugin->>Session: return appendSystemContext(XML)
Plugin->>Client: POST /sessions/init (fire-and-forget)
Plugin->>Client: POST /sessions/mark_injected (fire-and-forget)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 unit tests (beta)
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new capability to OpenClaw by integrating it with the Engram persistent memory server. The primary goal is to provide AI agents with long-term memory, allowing them to retain context, learn from past interactions, and make more informed decisions across sessions. This is achieved through a new TypeScript plugin that handles communication with Engram, manages context injection into prompts, and facilitates self-learning by ingesting tool call events and conversation transcripts. The changes also include necessary modifications to the core worker service to support agent-specific context resolution. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new OpenClaw plugin for persistent memory, which is a significant feature. The implementation includes a TypeScript plugin, a REST client, various tools, hooks, and commands, along with corresponding backend changes in Go to support agent identification. My review focuses on correctness, maintainability, and potential performance issues. I've identified some critical issues, such as invalid dependency versions that will break the build and a buggy circuit breaker implementation. Additionally, there are several opportunities to improve code quality by addressing code duplication, unsafe response handling, and potential performance bottlenecks. Overall, it's a comprehensive addition, but these key issues should be addressed before merging.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (3)
plugin/openclaw-engram/src/config.ts (1)
71-78: JSON Schema стоит синхронизировать с runtime-ограничениямиНа Line 71-75 сейчас
type: "number", но вPluginConfigSchemaте же поля —int().positive(). Добавьте в JSON Schematype: "integer"иminimum: 1, чтобы UI/валидация в манифесте совпадали с runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/config.ts` around lines 71 - 78, В JSON Schema object (the manifest schema) update the numeric fields to match the runtime PluginConfigSchema: change type: 'number' to type: 'integer' and add minimum: 1 for contextLimit, sessionContextLimit, tokenBudget and timeoutMs so UI/manifest validation matches the runtime int().positive() constraints referenced by PluginConfigSchema and keeps existing defaults and descriptions unchanged.plugin/openclaw-engram/src/tools/engram-remember.ts (1)
149-153: Есть дублирование правил title/content с/rememberЛогика формирования заголовка/усечения контента здесь пересекается с
plugin/openclaw-engram/src/commands/remember.ts(Line 55-63). Лучше вынести в общий util, чтобы исключить расхождение поведения.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/tools/engram-remember.ts` around lines 149 - 153, The title/content truncation logic in engram-remember.ts duplicates the same rules from commands/remember.ts (the parsed.data.text/parsed.data.content fallback and 80-char truncation), so extract that logic into a shared util (e.g., normalizeObservation or makeTitleAndContent) and replace the inline code in both engram-remember.ts and remember.ts to call the new util; ensure the util accepts the parsed.data object (or text and content) and returns { title, content } using the existing category-to-type mapping and truncation behavior so both locations use the single source of truth.plugin/openclaw-engram/openclaw.plugin.json (1)
10-59: Схема конфигурации продублирована в двух местах
configSchemaв манифесте иgetJsonSchema()вplugin/openclaw-engram/src/config.tsлегко разъедутся со временем. Лучше оставить один источник истины (генерация манифеста изgetJsonSchemaна этапе сборки или наоборот).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/openclaw.plugin.json` around lines 10 - 59, The manifest's duplicated configSchema should be eliminated and driven from the single source getJsonSchema() in plugin/openclaw-engram/src/config.ts: remove or replace the hard-coded "configSchema" in openclaw.plugin.json and update the build/process that produces the plugin manifest to import/serialize getJsonSchema() so the manifest's schema, enums, defaults and "required" array come directly from the getJsonSchema() function; ensure getJsonSchema() returns the same structure (including required: ["url","token"]) and update any packaging script to inject that JSON into the final openclaw.plugin.json during build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/openclaw-engram/package.json`:
- Around line 22-27: В package.json вы указали зависимость "@types/node":
"^25.5.0" но заявляете поддержку "node": ">=18.0.0", из‑за чего типы могут
ожидать API, отсутствующие в Node 18; откорректируйте версию `@types/node` на
соответствующую минимальной поддерживаемой версии Node (например к ветке 18.x —
`@types/node`@^18.x) или ограничьте "engines.node" до версии, соответствующей
`@types/node`, обновив значение в package.json; измените только поле
"dependencies/devDependencies" для "@types/node" или поле "engines"
соответственно и сохраните совместимость с TypeScript, проверив сборку.
In `@plugin/openclaw-engram/src/client.ts`:
- Around line 282-289: Функция extractOrigin обрезает path prefix из config.url
(например `/engram`) и возвращает только `https://host`, из‑за чего клиент
ломается за subpath; измените её так, чтобы при удачном парсинге возвращалось
протокол+хост И путь (т.е. include parsed.pathname, удалив только лишний
завершающий слэш), а в fallback для непарсируемых строк не отбрасывалась часть
пути (оставлять trimmed без .replace(/\/[^/]*$/, '')). Обновите extractOrigin
чтобы сохранять subpath из config.url и используйте эту новую строку в местах,
где используется config.url.
In `@plugin/openclaw-engram/src/config.ts`:
- Around line 9-12: Поле конфигурации url сейчас валидируется только через
z.string().url() и потому пропускает схемы отличные от HTTP/HTTPS; нужно
расширить валидацию поля url (тот самый url в config.ts, вызов z.string().url())
чтобы дополнительно проверять схему и допускать только http или https —
реализовать это добавлением refine-проверки или явной разбивки через конструктор
URL и проверку protocol === 'http:' || 'https:' и вернуть понятное сообщение об
ошибке, если схема некорректна.
In `@plugin/openclaw-engram/src/context/formatter.ts`:
- Around line 241-245: Текст напоминания в formatter.ts использует
несуществующее имя инструмента "decisions"; исправьте строку, чтобы она
упоминала зарегистрированное имя инструмента "engram_decisions" (в том месте где
формируется REMINDER вместе с упоминанием find_by_file) — найти и заменить
'decisions(query="...")' на 'engram_decisions(query="...")' (проверьте
функции/константы вокруг формирования строки в formatter.ts и согласуйте с
регистрацией в index.ts).
- Around line 181-193: В цикле по observations текущая проверка if (tokenCount +
tokens > tokenBudget && budgeted.length > 0) позволяет всегда добавлять первую
запись, даже если она превышает tokenBudget; замените условие на однозначное
ограничение (например if (tokenCount + tokens > tokenBudget) break;) либо, если
нужно пропускать одиночные слишком большие записи, пропустите их с continue
вместо добавления, но обязательно удалите зависимость от budgeted.length и
используйте переменные tokenCount, tokens, tokenBudget и массив budgeted в коде
в блоке форматтера (цикл, где рассчитываются title, narrative, facts).
In `@plugin/openclaw-engram/src/hooks/after-tool-call.ts`:
- Around line 36-46: The JSON.stringify calls for toolInput/toolResult can throw
(cycles, BigInt) and the fire-and-forget client.ingestEvent is sent with void
which can cause unhandled rejections; wrap serialization in a safe serializer
(e.g., safeSerialize(event.toolInput) and safeSerialize(event.toolResult)) that
falls back to a non-throwing string (or uses a replacer/try-catch) before
passing to truncate(…, TOOL_INPUT_MAX_CHARS/TOOL_RESULT_MAX_CHARS), and send the
ingest as fire-and-forget but attach an error handler (e.g.,
client.ingestEvent(...).catch(err => processLogger.error(...))) instead of using
bare void so failures from client.ingestEvent are caught; update references:
toolInput, toolResult, truncate, TOOL_INPUT_MAX_CHARS, TOOL_RESULT_MAX_CHARS,
client.ingestEvent, event.toolInput, event.toolResult, event.toolName, agentId,
project.
In `@plugin/openclaw-engram/src/hooks/before-prompt-build.ts`:
- Around line 40-77: The REST client calls in before-prompt-build (notably
client.searchContext(...) and client.initSession(...)) are not protected from
network/server errors and the fire-and-forget client.markInjected(...) lacks a
.catch; wrap the await calls in try/catch blocks (capture/log the error via
logger or console) and only proceed when the call succeeds, and change the
fire-and-forget call to void client.markInjected(...).catch(err => { (logger ??
console).warn(..., err) }); apply the same pattern to other hooks using the same
symbols (client.searchContext, client.initSession, client.markInjected) so
errors are swallowed/logged and do not break the hook lifecycle.
In `@plugin/openclaw-engram/src/hooks/session-end.ts`:
- Around line 32-55: The current code passes agentId into client.backfillSession
as session_id which conflates multiple sessions for the same agent; extract the
real session id from the event (e.g. const sessionId = event.sessionId ??
event.session?.id) and use that when calling client.backfillSession, adding a
guard to return (or skip backfill) if sessionId is falsy so you don't fall back
to agentId accidentally; update the call site in client.backfillSession (replace
session_id: agentId) and keep agentId only for identity resolution where it
belongs.
In `@plugin/openclaw-engram/src/hooks/session-start.ts`:
- Around line 62-66: Инициализация сессии в вызове client.initSession использует
agentId для поля claudeSessionId, из‑за чего все диалоги одного агента мапятся в
одну сессию; замените передачу agentId на уникальный идентификатор сессии
(например event.sessionId или сгенерированный sessionId) при вызове
client.initSession({ claudeSessionId: ..., project, prompt: event.initialPrompt
}); — найдите вызов client.initSession в hooks/session-start.ts и подставьте
корректное поле/переменную вместо agentId, чтобы каждый новый диалог создавал
отдельную сессию на стороне engram.
In `@plugin/openclaw-engram/src/identity.ts`:
- Around line 53-58: The projectId currently prefixes the hash with the local
directory name (basename(resolve(cwd))) which makes IDs vary across checkouts;
change projectId to be derived only from the stable key (remoteURL + '/' +
relativePath) / hash so it is identical across clones — e.g. remove the dirName
prefix and return projectId based solely on hash.slice(0,8) (keep gitRemote =
remoteURL unchanged); update the code around key, hash and projectId to reflect
this (refer to projectId, key, hash, remoteURL, relativePath, cwd in the diff).
- Around line 43-52: getGitRemoteID synchronously runs two execSync calls on
every invocation (used by resolveIdentity), which blocks hot paths; fix by
adding memoization keyed by cwd: create a module-level Map<string,
GitRemoteResult | null>, have getGitRemoteID check the cache first, only run the
execSync calls if missing, and store the resulting GitRemoteResult (or null) in
the cache before returning; keep the same return shape and use the cached value
from resolveIdentity so repeated calls for the same workspace do not re-run
execSync.
In `@plugin/openclaw-engram/src/index.ts`:
- Around line 133-160: The code defaults to the literal project id 'cli' which
mixes CLI memories across workspaces; remove that hardcoded fallback and use the
actual configured project (or undefined) so project resolution stays
workspace-specific: update both the client.searchContext call (where project:
config.project ?? 'cli') and the client.bulkImport payload (where project:
config.project ?? 'cli') to use project: config.project ?? undefined (or simply
project: config.project) so no global 'cli' scope is injected; ensure
memCmd.command('store') and the memory search path use the same project
resolution logic.
In `@plugin/openclaw-engram/src/tools/engram-search.ts`:
- Around line 13-20: The parsed `limit` from SearchParamsSchema/searchParameters
is currently ignored: after safeParse() the code never passes the limit to
searchContext() nor applies it before formatContext(), so calls with limit:1
still return all observations; fix by extracting limit from the result of
safeParse(), pass that limit into searchContext() (or use it to slice the
returned results) and enforce the same limit when calling formatContext() (or
before token-truncation) in the functions that call
searchContext()/formatContext() so only up to `limit` items are returned and
formatted; update references in the same module where safeParse(),
searchContext(), formatContext(), SearchParamsSchema and searchParameters are
used.
In `@plugin/openclaw-engram/src/tools/memory-get.ts`:
- Around line 61-68: The memory_get tool currently allows reading arbitrary
workspace files because readLocalFile lacks extension checks; update
readLocalFile (used by memory_get) to validate the resolved file path ends with
the allowed markdown extensions (e.g., .md and .markdown) after calling
api.resolvePath, and reject or return a safe error message for other extensions
(do not read or expose content of .env, keys, etc.); ensure the check is
case-insensitive and keep existing empty-file handling and error paths intact.
---
Nitpick comments:
In `@plugin/openclaw-engram/openclaw.plugin.json`:
- Around line 10-59: The manifest's duplicated configSchema should be eliminated
and driven from the single source getJsonSchema() in
plugin/openclaw-engram/src/config.ts: remove or replace the hard-coded
"configSchema" in openclaw.plugin.json and update the build/process that
produces the plugin manifest to import/serialize getJsonSchema() so the
manifest's schema, enums, defaults and "required" array come directly from the
getJsonSchema() function; ensure getJsonSchema() returns the same structure
(including required: ["url","token"]) and update any packaging script to inject
that JSON into the final openclaw.plugin.json during build.
In `@plugin/openclaw-engram/src/config.ts`:
- Around line 71-78: В JSON Schema object (the manifest schema) update the
numeric fields to match the runtime PluginConfigSchema: change type: 'number' to
type: 'integer' and add minimum: 1 for contextLimit, sessionContextLimit,
tokenBudget and timeoutMs so UI/manifest validation matches the runtime
int().positive() constraints referenced by PluginConfigSchema and keeps existing
defaults and descriptions unchanged.
In `@plugin/openclaw-engram/src/tools/engram-remember.ts`:
- Around line 149-153: The title/content truncation logic in engram-remember.ts
duplicates the same rules from commands/remember.ts (the
parsed.data.text/parsed.data.content fallback and 80-char truncation), so
extract that logic into a shared util (e.g., normalizeObservation or
makeTitleAndContent) and replace the inline code in both engram-remember.ts and
remember.ts to call the new util; ensure the util accepts the parsed.data object
(or text and content) and returns { title, content } using the existing
category-to-type mapping and truncation behavior so both locations use the
single source of truth.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 880d257c-7ebf-4163-9182-d4528a4e77db
📒 Files selected for processing (24)
internal/worker/handlers_context.goplugin/openclaw-engram/.gitignoreplugin/openclaw-engram/openclaw.plugin.jsonplugin/openclaw-engram/package.jsonplugin/openclaw-engram/src/availability.tsplugin/openclaw-engram/src/client.tsplugin/openclaw-engram/src/commands/memory.tsplugin/openclaw-engram/src/commands/remember.tsplugin/openclaw-engram/src/config.tsplugin/openclaw-engram/src/context/formatter.tsplugin/openclaw-engram/src/hooks/after-tool-call.tsplugin/openclaw-engram/src/hooks/before-compaction.tsplugin/openclaw-engram/src/hooks/before-prompt-build.tsplugin/openclaw-engram/src/hooks/session-end.tsplugin/openclaw-engram/src/hooks/session-start.tsplugin/openclaw-engram/src/identity.tsplugin/openclaw-engram/src/index.tsplugin/openclaw-engram/src/tools/engram-decisions.tsplugin/openclaw-engram/src/tools/engram-remember.tsplugin/openclaw-engram/src/tools/engram-search.tsplugin/openclaw-engram/src/tools/memory-forget.tsplugin/openclaw-engram/src/tools/memory-get.tsplugin/openclaw-engram/src/types/openclaw.tsplugin/openclaw-engram/tsconfig.json
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
plugin/openclaw-engram/src/index.ts (1)
157-189:⚠️ Potential issue | 🟠 MajorCLI всё ещё пишет и ищет память в общем проекте
cli.Эта проблема уже поднималась раньше и всё ещё видна в текущем коде: hooks/tools вычисляют
projectиз workspace, а CLIsearch/storeпродолжают падать в literal'cli'. В результате память разных репозиториев смешивается, а записи, сделанные из CLI, не совпадают с project scope обычного рантайма.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/index.ts` around lines 157 - 189, The CLI commands memCmd.command('search') (client.searchContext) and memCmd.command('store') (client.bulkImport) currently fall back to the hardcoded project literal 'cli' via config.project ?? 'cli'; change them to use the workspace-derived project resolution the runtime/hooks/tools use instead of the 'cli' default—i.e. obtain the project id the same way the rest of the codebase does (use the existing workspace/project helper or the resolved config.project without defaulting to 'cli') and pass that value into client.searchContext and client.bulkImport so CLI storage/search operate in the correct per-workspace project.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/openclaw-engram/src/tools/memory-migrate.ts`:
- Around line 91-93: The code currently sets marker = params.force ? null :
await loadMarker(markerPath), which causes the existing marker state to be
discarded when running with --force and leads to wiping migrated-file state
(only newFileHashes get persisted); always call await loadMarker(markerPath) to
load existing marker regardless of params.force, and change the logic that uses
params.force to only bypass per-file skip checks (i.e., use params.force when
deciding to skip hashing/compare, but do not set marker to null). Update all
similar occurrences (where params.force currently nulls the marker) so
saveMarker/write of hashes merges with the loaded marker rather than replacing
it with only newFileHashes.
- Around line 167-184: Не сохраняйте marker при частичных/полных ошибках
импорта: в функции/блоке где вызывается client.bulkImport(batch) (работа с
observations, response, errors, totalImported/totalSkipped) нужно накопить
информацию о том, какие файлы/батчи реально успешно импортированы и только их
хэши включать в новый MigrationMarker; если какой‑либо response равен null или
response.errors не пуст — не записывать весь newFileHashes и/или вовсе
пропустить вызов saveMarker(markerPath, updatedMarker). Лучший простой фикс —
собирать список успешно импортированных файлов при обработке каждого batch и
формировать updatedMarker.files из этого списка (или отказаться от saveMarker
при errors.length>0); используйте существующие идентификаторы newFileHashes,
marker, MigrationMarker и saveMarker при правке.
---
Duplicate comments:
In `@plugin/openclaw-engram/src/index.ts`:
- Around line 157-189: The CLI commands memCmd.command('search')
(client.searchContext) and memCmd.command('store') (client.bulkImport) currently
fall back to the hardcoded project literal 'cli' via config.project ?? 'cli';
change them to use the workspace-derived project resolution the
runtime/hooks/tools use instead of the 'cli' default—i.e. obtain the project id
the same way the rest of the codebase does (use the existing workspace/project
helper or the resolved config.project without defaulting to 'cli') and pass that
value into client.searchContext and client.bulkImport so CLI storage/search
operate in the correct per-workspace project.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 736b3ab4-4daa-427f-b434-674251360b78
📒 Files selected for processing (3)
plugin/openclaw-engram/openclaw.plugin.jsonplugin/openclaw-engram/src/index.tsplugin/openclaw-engram/src/tools/memory-migrate.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/openclaw-engram/openclaw.plugin.json
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
plugin/openclaw-engram/src/tools/memory-migrate.ts (2)
91-93:⚠️ Potential issue | 🟠 Major
forceпо-прежнему затирает marker для остальных файлов.При
force=truemarker устанавливается вnull, и на строке 204marker?.files ?? {}становится{}. Это приводит к потере состояния уже мигрированных файлов, не входящих в текущий запуск. Следующий обычный запуск повторно импортирует весь workspace.Рекомендация из предыдущего ревью: всегда загружать marker, а
forceиспользовать только для обхода skip-проверки.💡 Рекомендуемое исправление
- const marker = params.force ? null : await loadMarker(markerPath); + const marker = await loadMarker(markerPath);И в проверке пропуска:
- if (marker?.files[relPath] === hash) { + if (!params.force && marker?.files[relPath] === hash) { skippedFiles.push(relPath); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/tools/memory-migrate.ts` around lines 91 - 93, The current code sets marker = params.force ? null : await loadMarker(...), which wipes existing marker state when force=true; instead always load the marker via loadMarker(markerPath) and keep its data (use the existing marker variable as returned), and change the skip-check logic (the places that use marker?.files ?? {}) to consider params.force as an override only for skipping per-file checks (i.e., if params.force is true bypass the "already migrated" skip, but do not discard marker contents); update references to marker (e.g., markerPath, loadMarker, marker?.files) so marker is never nullified by force and force only toggles the skip condition.
168-175:⚠️ Potential issue | 🟠 MajorДлинные секции по-прежнему тихо обрезаются.
Контент режется до 900 символов (строка 170), но marker сохраняет хеш полного файла (строка 123). Хвост секции не попадает в engram и не будет повторно обработан без
force.Рекомендация: либо дробить oversized chunk на несколько observation, либо не считать файл полностью мигрированным при наличии усечённых чанков.
🧹 Nitpick comments (2)
plugin/openclaw-engram/src/tools/memory-migrate.ts (2)
357-363: Тихое подавление ошибок при сохранении marker.При неудачной записи marker миграция завершается успешно, но состояние не сохраняется. Следующий запуск повторит импорт. Рекомендуется хотя бы логировать предупреждение.
♻️ Опциональное улучшение
async function saveMarker(path: string, marker: MigrationMarker): Promise<void> { try { await writeFile(path, JSON.stringify(marker, null, 2), 'utf-8'); - } catch { - // Non-critical — migration still succeeded + } catch (err) { + // Non-critical — migration still succeeded, but warn user + console.warn(`[memory_migrate] Failed to save marker: ${err}`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/tools/memory-migrate.ts` around lines 357 - 363, The saveMarker function currently swallows all errors from writeFile, losing state silently; modify saveMarker to catch the exception as an error variable and log a warning including the target path and error details (e.g., "Failed to save migration marker" + path + error.message) so the failure is visible while keeping the non-fatal behavior; use the project's existing logger if available (or console.warn if not) and retain the function signature and use of MigrationMarker/writeFile.
281-285: Обработка fenced code blocks — хороший фикс.Логика
inFencetoggle корректно предотвращает ложные разбиения по##внутри code blocks.Потенциальный edge case: вложенные fence-блоки (например, четыре backtick'а содержащие три) могут сбить счётчик, но для типичных memory-файлов это маловероятно.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/tools/memory-migrate.ts` around lines 281 - 285, The current inFence toggle flips whenever a line.startsWith('```') which misbehaves for nested or mismatched fence lengths; instead, record the fence delimiter string when entering a fence (e.g., capture the leading backtick run) and only exit the fence when a line matches that same delimiter, updating the logic around the inFence variable inside the for (const line of lines) loop to use a currentFence/token rather than a simple boolean toggle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/openclaw-engram/src/tools/memory-migrate.ts`:
- Around line 97-106: The current startsWith check is vulnerable to
sibling-directory traversal (e.g., "/home/user/project-evil" passes). Replace
the startsWith check with a proper path-boundary check using path.relative (or
equivalent): compute rel = path.relative(normalizedWs, resolved) and reject when
rel === ''? Actually reject when rel.startsWith('..') or path.isAbsolute(rel)
(i.e., when resolved lies outside workspaceDir); accept only when rel does not
start with '..' (and does not contain path separators leading '..' segments).
Update the conditional that uses resolved, normalizedWs, normalize, resolve,
api.resolvePath and workspaceDir to use this relative-based validation so only
true descendants of workspaceDir are allowed.
---
Duplicate comments:
In `@plugin/openclaw-engram/src/tools/memory-migrate.ts`:
- Around line 91-93: The current code sets marker = params.force ? null : await
loadMarker(...), which wipes existing marker state when force=true; instead
always load the marker via loadMarker(markerPath) and keep its data (use the
existing marker variable as returned), and change the skip-check logic (the
places that use marker?.files ?? {}) to consider params.force as an override
only for skipping per-file checks (i.e., if params.force is true bypass the
"already migrated" skip, but do not discard marker contents); update references
to marker (e.g., markerPath, loadMarker, marker?.files) so marker is never
nullified by force and force only toggles the skip condition.
---
Nitpick comments:
In `@plugin/openclaw-engram/src/tools/memory-migrate.ts`:
- Around line 357-363: The saveMarker function currently swallows all errors
from writeFile, losing state silently; modify saveMarker to catch the exception
as an error variable and log a warning including the target path and error
details (e.g., "Failed to save migration marker" + path + error.message) so the
failure is visible while keeping the non-fatal behavior; use the project's
existing logger if available (or console.warn if not) and retain the function
signature and use of MigrationMarker/writeFile.
- Around line 281-285: The current inFence toggle flips whenever a
line.startsWith('```') which misbehaves for nested or mismatched fence lengths;
instead, record the fence delimiter string when entering a fence (e.g., capture
the leading backtick run) and only exit the fence when a line matches that same
delimiter, updating the logic around the inFence variable inside the for (const
line of lines) loop to use a currentFence/token rather than a simple boolean
toggle.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7d9459c-441b-4c60-8a2d-7710a9685fb0
📒 Files selected for processing (1)
plugin/openclaw-engram/src/tools/memory-migrate.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
plugin/openclaw-engram/src/tools/memory-migrate.ts (2)
107-113:⚠️ Potential issue | 🟠 MajorПроверка пути через
startsWithнебезопасна и допускает обход через sibling/симлинки.Текущая валидация пропускает
.../workspace-evil/...и не защищает от симлинков, указывающих за пределы workspace.🛡️ Рекомендуемая правка проверки границ workspace
-import { readFile, readdir, writeFile, rename, stat, lstat } from 'node:fs/promises'; +import { readFile, readdir, writeFile, rename, stat, lstat, realpath } from 'node:fs/promises'; -import { join, relative, resolve, normalize } from 'node:path'; +import { join, relative, resolve, normalize, isAbsolute } from 'node:path'; ... if (params.path) { - const resolved = normalize(resolve(api.resolvePath(params.path))); - const normalizedWs = normalize(resolve(workspaceDir)); - if (!resolved.startsWith(normalizedWs)) { + const resolved = normalize(resolve(api.resolvePath(params.path))); + const normalizedWs = normalize(resolve(workspaceDir)); + const [resolvedReal, wsReal] = await Promise.all([ + realpath(resolved), + realpath(normalizedWs), + ]); + const rel = relative(wsReal, resolvedReal); + const st = await lstat(resolved); + if (st.isSymbolicLink() || rel.startsWith('..') || isAbsolute(rel)) { return `Path "${params.path}" resolves outside the workspace — access denied.`; } - filePaths = [resolved]; + filePaths = [resolvedReal]; } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/tools/memory-migrate.ts` around lines 107 - 113, The current check using resolved.startsWith(normalizedWs) in memory-migrate.ts is unsafe; replace it with a realpath-based containment check to prevent sibling and symlink escapes: resolve the absolute path and the workspaceDir using fs.realpath (or fs.realpathSync) to get their canonical paths (referenced variables: params.path, resolved, workspaceDir, normalizedWs), then compute path.relative(canonicalWorkspace, canonicalResolved) and ensure it does not start with '..' or equal '..' (also guard with path.sep where appropriate); if the resolved path is outside, return the same access-denied message, otherwise assign filePaths = [resolved].
103-104:⚠️ Potential issue | 🟠 Major
forceсейчас сбрасывает marker-состояние для остальных файлов.При
force=truemarker не загружается, а затем перезаписывается толькоnewFileHashes. Для частичной миграции (path) это стирает историю уже мигрированных файлов вне выбранного пути.💡 Минимальный безопасный фикс
- const marker = params.force ? null : await loadMarker(markerPath); + const marker = await loadMarker(markerPath); ... - if (marker?.files[relPath] === hash) { + if (!params.force && marker?.files?.[relPath] === hash) { skippedFiles.push(relPath); continue; }Also applies to: 136-137, 212-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/tools/memory-migrate.ts` around lines 103 - 104, При текущей логике params.force=true пропускает загрузку marker (const marker = params.force ? null : await loadMarker(markerPath)) и потом при записи заменяются только newFileHashes, что стирает историю вне указанного path; исправьте так: всегда загрузите существующий marker via loadMarker(markerPath), and if params.force is true and params.path is provided, only clear/overwrite entries within that path (remove keys matching params.path) before merging newFileHashes, otherwise if params.force and no path then clear the entire marker; ensure merge logic updates marker entries (marker = { ...marker, ...newFileHashes } or equivalent) rather than replacing the whole object so files outside the target scope keep their state (reference symbols: params.force, params.path, loadMarker(markerPath), marker, newFileHashes).plugin/openclaw-engram/src/index.ts (1)
162-165:⚠️ Potential issue | 🟠 MajorНе используйте глобальный fallback
project: 'cli'в CLI-командах.Это смешивает записи между репозиториями и расходится с workspace-ориентированной идентификацией в остальном плагине.
💡 Консистентный вариант
+import { projectIDFromWorkspace } from './identity.js'; ... memCmd .command('search') .description('Search engram memory') .argument('<query>', 'Search query') .action(async (query: unknown) => { + const project = config.project ?? projectIDFromWorkspace(process.cwd()); const response = await client.searchContext({ - project: config.project ?? 'cli', + project, query: String(query), }); ... memCmd .command('store') .description('Store a memory') .argument('<text>', 'Text to remember') .action(async (text: unknown) => { + const project = config.project ?? projectIDFromWorkspace(process.cwd()); const textStr = String(text); const title = textStr.length > 80 ? textStr.slice(0, 77) + '...' : textStr; const response = await client.bulkImport([{ title, content: textStr.slice(0, 900), type: 'context', - project: config.project ?? 'cli', + project, scope: 'project', }]);Also applies to: 184-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/index.ts` around lines 162 - 165, Remove the hardcoded fallback project: stop passing project: config.project ?? 'cli' to client.searchContext and instead require/validate config.project is present (or derive it from the workspace context where the rest of the plugin gets its workspace id); replace the fallback expression with a direct use of config.project and add a guard that throws a clear error if config.project is missing. Apply this change to the client.searchContext call at the shown snippet and the similar occurrence around the 184-189 block so both locations no longer default to 'cli'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@plugin/openclaw-engram/src/index.ts`:
- Around line 162-165: Remove the hardcoded fallback project: stop passing
project: config.project ?? 'cli' to client.searchContext and instead
require/validate config.project is present (or derive it from the workspace
context where the rest of the plugin gets its workspace id); replace the
fallback expression with a direct use of config.project and add a guard that
throws a clear error if config.project is missing. Apply this change to the
client.searchContext call at the shown snippet and the similar occurrence around
the 184-189 block so both locations no longer default to 'cli'.
In `@plugin/openclaw-engram/src/tools/memory-migrate.ts`:
- Around line 107-113: The current check using resolved.startsWith(normalizedWs)
in memory-migrate.ts is unsafe; replace it with a realpath-based containment
check to prevent sibling and symlink escapes: resolve the absolute path and the
workspaceDir using fs.realpath (or fs.realpathSync) to get their canonical paths
(referenced variables: params.path, resolved, workspaceDir, normalizedWs), then
compute path.relative(canonicalWorkspace, canonicalResolved) and ensure it does
not start with '..' or equal '..' (also guard with path.sep where appropriate);
if the resolved path is outside, return the same access-denied message,
otherwise assign filePaths = [resolved].
- Around line 103-104: При текущей логике params.force=true пропускает загрузку
marker (const marker = params.force ? null : await loadMarker(markerPath)) и
потом при записи заменяются только newFileHashes, что стирает историю вне
указанного path; исправьте так: всегда загрузите существующий marker via
loadMarker(markerPath), and if params.force is true and params.path is provided,
only clear/overwrite entries within that path (remove keys matching params.path)
before merging newFileHashes, otherwise if params.force and no path then clear
the entire marker; ensure merge logic updates marker entries (marker = {
...marker, ...newFileHashes } or equivalent) rather than replacing the whole
object so files outside the target scope keep their state (reference symbols:
params.force, params.path, loadMarker(markerPath), marker, newFileHashes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c5c2378-a5c1-4ccf-a6a4-be486c2024e9
📒 Files selected for processing (2)
plugin/openclaw-engram/src/index.tsplugin/openclaw-engram/src/tools/memory-migrate.ts
Allow OpenClaw agents (which lack filesystem context) to use agent_id as project scope in handleSearchByPrompt and handleContextInject. Backward-compatible: existing Claude Code hooks using project param are unaffected.
TypeScript plugin connecting OpenClaw's AI gateway to engram via REST API.
Core features:
- Dual-tier context injection (session-level static + per-turn dynamic)
- Automatic self-learning via tool event ingestion and transcript backfill
- 3-strike passive availability detection with 60s cooldown
- agentId-first identity with git remote fallback
- Zod-validated config with JSON Schema export
Hooks: session_start, before_prompt_build, after_tool_call,
before_compaction, session_end
Tools: engram_search, engram_remember, engram_decisions
Commands: /memory (status), /remember (quick store)
Context formatter ported from plugin/engram/hooks/user-prompt.js:
credential filter, Jaccard title dedup, type grouping, token budget,
XML-tag rendering.
- Rewrite types/openclaw.ts with verified SDK shapes: register(api), AnyAgentTool, ToolFactory, PluginLogger, 24 hook names, CliProgram - Rewrite index.ts: id-based plugin, api.on() hooks, tool factory pattern, registerCli for 'memory' CLI subcommands - Refactor 3 tool files to factory pattern with TypeBox schemas - Add memory_search (alias), memory_store (text/content compat), memory_forget (bulk delete), memory_get (dual-mode: fs + engram) - Update 5 hooks: optional event fields, PluginLogger injection - Update 2 commands: OpenClawPluginCommandDefinition, name without slash - Add @sinclair/typebox dep, move zod to devDependencies - Add bulkDelete method to EngramRestClient - Version bump to 0.2.0, manifest updated with 7 tools + CLI section
Adds memory_migrate tool, /migrate slash command, and `openclaw memory migrate` CLI subcommand. Discovers MEMORY.md + memory/**/*.md, splits by ## headers, bulk-imports into engram with SHA256-based idempotency marker.
- Path traversal: validate resolved path stays within workspaceDir - Marker idempotency: only save marker on full success, not partial failure - Symlink safety: skip symlinks + max depth guard in findMdFiles - Code fence awareness: track fenced blocks to avoid false ## splits - Truncation visibility: warn about content truncation in dry-run and report
- Schema drift guard: compile-time check that Zod and TypeBox field names match - Marker race condition: atomic write via temp file + rename - CLI tool caching: reuse migrate tool instance instead of recreating per call
…xtensions entry
OpenClaw discovery ignores package.json "main" field. It uses:
1. package.json "openclaw.extensions" array for entry point resolution
2. Fallback to index.ts/index.js in root
Added "openclaw": { "extensions": ["dist/index.js"] } and renamed
package name from "openclaw-engram-plugin" to "engram" so idHint
matches manifest id and config key.
Bug 1: register() was async — OpenClaw ignores Promise returns from
register(), logging "async registration is ignored". Made synchronous.
Bug 2: Commands used { execute(args, ctx) } shape but SDK expects
{ handler(ctx) } where handler is typeof "function". SDK checks
`typeof command.handler !== "function"` and rejects with
"Command handler must be a function".
Also updated PluginCommandContext and PluginCommandResult types
to match real SDK (ctx.args is raw string, result uses { text }
not { output }).
…ncation Extract splitIntoChunks, inferType, loadMarker, saveMarker, discoverMemoryFiles, safeReadFile, fileExists into utils/memory-files.ts for reuse by both memory-migrate tool and file-watcher service. Remove artificial 900-char content truncation on import — engram server stores full TEXT without limits. Fix BUG #4: dry run output now shows summary + first 5 chunks instead of flooding with all chunks.
Add chokidar-based FileWatcherService that watches MEMORY.md and memory/**/*.md in the agent workspace. On file changes, debounces 1500ms, computes SHA256 hash, and bulk-imports new/changed chunks into engram. Reuses shared chunking and marker logic from utils/memory-files.ts. Replaces memory-core's built-in chokidar watcher, enabling engram to fully occupy the exclusive memory plugin slot. Review fixes applied: - stopped flag prevents callbacks after stop() - inFlight collision reschedules instead of dropping events - projectId cached in constructor (avoids git execSync per sync)
Engram API only accepts: bugfix, feature, refactor, discovery, decision, change. The plugin was using 'context' as default type which caused bulk-import to reject observations with "invalid type 'context'".
- extractOrigin preserves URL subpath (client.ts) - Replace hardcoded 'cli' project fallback with workspace-derived ID - force flag no longer wipes migration marker state (memory-migrate.ts) - Path traversal protection via relative() instead of startsWith - Use event.sessionId instead of agentId for session tracking - Enforce search limit parameter (engram-search.ts) - Restrict memory_get to .md files only (memory-get.ts) - Safe serialization + .catch() for fire-and-forget calls - Fix tool name references in formatter reminder text - Validate URL scheme is http/https (config.ts) - Log warning on marker save failure (memory-files.ts)
thebtf
force-pushed
the
feature/openclaw-engram-plugin
branch
from
c521d0e to
99a3fe8
Compare
Owner
Author
|
@codex review |
- package.json: align @types/node to ^18.19.0 to match engines.node >=18
- client.ts: return null instead of {} for empty response body; fix extractOrigin fallback to preserve path segments
- identity.ts: add module-level memoization cache for getGitRemoteID to avoid blocking execSync on hot paths; derive projectId from remote URL (not local dir name) for stable cross-checkout IDs
- formatter.ts: fix token budget bypass — oversized observations now skipped with continue instead of always inserting the first entry
- before-prompt-build.ts: wrap searchContext in try/catch to prevent hook lifecycle disruption on network errors
- session-end.ts: use event.sessionId only (no agentId fallback) to avoid conflating backfill across sessions
- availability.ts: fix recordSuccess wasUnavailable detection (use unavailableSince directly, not isAvailable); fix recordFailure to re-trip circuit in half-open state
- openclaw.plugin.json: add uiHints sensitive:true to token field
6 tasks
7 tasks
thebtf
added a commit
that referenced
this pull request
Apr 15, 2026
TestLRU_NewKey: new key returns false TestLRU_DuplicateKey: duplicate returns true TestLRU_Eviction_AtCapacity: capacity-4 LRU evicts on insert #5 TestLRU_LeastRecentEvicted: refreshed entry survives, unreferenced entry evicted TestLRU_ConcurrentMark: 100 goroutines racing on unique + shared keys
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.
Summary
plugin/openclaw-engram/— TypeScript plugin connecting OpenClaw agents to engram's persistent memory server via REST APIagent_idparam in context endpoints for plugin identity resolutionregister(api), tool factory pattern,api.on()hooks, TypeBox schemas,registerCliPlugin capabilities
engram_search,engram_remember,engram_decisions,memory_search(alias),memory_store(text/content compat),memory_forget(bulk delete),memory_get(dual-mode: local fs + engram)session_start(static context),before_prompt_build(per-turn search),after_tool_call(self-learning),before_compaction/session_end(transcript backfill)/memory(status),/remember(quick store)openclaw memory status|search|storeArchitecture
Plugin is a REST client only — all heavy lifting (embedding, search, scoring, extraction) happens on the engram server. No local DB, no local models.
Test plan
tsc --noEmitpasses with strict modeSummary by CodeRabbit
Новые возможности
Исправления
Chores