v0.5.0: vault integration, legacy cleanup, chuck features#9
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
ОбзорPR вносит существенные изменения: удаляет локальную верификацию через Claude CLI, добавляет отслеживание нулевых результатов поиска с хранилищем в БД, внедряет извлечение и хранение секретов через vault, вводит управление контекстом заполнения для бюджета токенов, обновляет API с GET на POST для впрыски контекста и добавляет новые аналитические эндпоинты. Изменения
Диаграмма последовательностиsequenceDiagram
participant Client as Клиент
participant Handler as handleSearchByPrompt
participant SearchMgr as searchMgr
participant DB as ObservationStore
participant Tracker as trackSearchMiss
participant BackendDB as Backend DB
Client->>Handler: POST /api/context/inject<br/>(query, project)
Handler->>SearchMgr: Clustered search
SearchMgr->>DB: Query observations
DB-->>SearchMgr: [] (пусто)
alt Нулевые результаты
Handler->>Tracker: trackSearchMiss(project, query)
Tracker->>BackendDB: RecordSearchMiss(project, query)
BackendDB-->>Tracker: ✓
else Есть результаты
SearchMgr-->>Handler: observations[]
end
Handler-->>Client: JSON response
sequenceDiagram
participant Client as Клиент (backfill)
participant Handler as handleBackfillIngest
participant Privacy as privacy.ExtractSecrets
participant VaultUtil as vaultStoreDetectedSecrets
participant Vault as Vault (crypto)
participant ObsStore as ObservationStore
participant Backend as Backend DB
Client->>Handler: Raw content
Handler->>Privacy: ExtractSecrets(content)
Privacy-->>Handler: []DetectedSecret
Handler->>VaultUtil: vaultStoreDetectedSecrets(ctx, text, project)
VaultUtil->>Vault: Encrypt secrets
Vault-->>VaultUtil: encrypted values
VaultUtil->>ObsStore: CreateObservation(credential)
ObsStore->>Backend: INSERT
Backend-->>ObsStore: ✓
VaultUtil-->>Handler: logged summary
Handler->>Privacy: RedactSecrets(content)
Privacy-->>Handler: redacted content
Handler-->>Client: processed session
Оценка усилий на проверку кода🎯 4 (Complex) | ⏱️ ~50 минут Возможно связанные PR
Рекомендуемые метки
Стихотворение
🚥 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)
Comment |
Owner
Author
|
@coderabbitai review |
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 marks a significant update to version 0.5.0, focusing on enhancing the system's intelligence, security, and maintainability. It introduces robust secret management by integrating a vault for server-side secret detection and storage, improving data privacy. The context injection mechanism is made more dynamic with context-fill-aware throttling, and a new contradiction detection feature helps refine agent behavior. Furthermore, the system now tracks search misses to enable self-tuning retrieval, and several legacy components and dead code have been removed to streamline the codebase and improve performance. Highlights
Changelog
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
|
Owner
Author
|
@gemini-code-assist review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@codex review |
![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 is an impressive update that introduces several significant features and cleans up a substantial amount of legacy code. The server-side secret detection with vault integration is a major security and architecture improvement. The removal of the local Claude CLI fallback and related configuration simplifies the codebase nicely. I also appreciate the new intelligent features like context-fill-aware injection throttling and the experimental contradiction detection. The implementation is solid across the board. I have one minor suggestion to improve the logging for the new contradiction detection feature.
There was a problem hiding this comment.
Code Review
This pull request introduces several significant features and refactorings. Key additions include server-side secret detection with Vault integration for secure credential storage, context-fill-aware injection throttling inspired by 'chuck', and contradiction detection in the after_tool_call hook. The PR also adds analytics for tracking search misses and introduces new API endpoints for decision search and search miss analytics.
A substantial part of this PR is dedicated to cleanup and refactoring, including the removal of legacy local-mode support (Claude CLI fallback) and other dead code, which simplifies the configuration and codebase. The context/inject endpoint has been migrated from GET to POST for better handling of larger payloads.
My review focuses on improving code structure and consistency. I've suggested a refactoring to reduce code duplication in one of the HTTP handlers and an enhancement to logging in the new contradiction detection feature. Overall, the changes are well-structured and align with the goals outlined in the pull request description.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/worker/sdk/processor.go (1)
639-645:⚠️ Potential issue | 🟠 MajorОтвяжите timeout от context.Background().
Функция
callLLMполучает параметрctxот вызывающей стороны, но игнорирует его при создании контекста для LLM API. Использованиеcontext.WithTimeout(context.Background(), 120*time.Second)означает, что отмена родительского контекста (например, при завершении запроса или shutdown воркера) не повлияет на вызов LLM. После удаления fallback на Claude CLI это единственный путь к LLM, поэтому такие потерянные отмены будут держать сетевые соединения и семафоры до 120 секунд дольше необходимого.Таймаут нужно применять поверх входного контекста:
Предлагаемое исправление
- llmCtx, cancel := context.WithTimeout(context.Background(), 120*time.Second) + llmCtx, cancel := context.WithTimeout(ctx, 120*time.Second)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/sdk/processor.go` around lines 639 - 645, The loop in callLLM creates llmCtx from context.Background(), ignoring the incoming ctx; change llmCtx creation to use the caller's ctx (llmCtx, cancel := context.WithTimeout(ctx, 120*time.Second)) so parent cancellations propagate to LLM calls, and ensure cancel() is called immediately after the p.llmClient.Complete call (do not use defer inside the retry loop to avoid accumulating defers). Also check ctx.Err() between retries and break/return early if the parent context is canceled; adjust references to llmCtx, cancel, and the p.llmClient.Complete call accordingly.
🧹 Nitpick comments (4)
plugin/openclaw-engram/src/hooks/before-compaction.ts (1)
49-52: Стоит вынести strip/truncate в общий helper между хуками.Логика обработки контента здесь и в
plugin/openclaw-engram/src/hooks/session-end.tsпрактически одинаковая; лучше централизовать, чтобы избежать расхождения поведения при следующих изменениях.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/hooks/before-compaction.ts` around lines 49 - 52, Extract the repeated strip-and-truncate logic into a shared helper (e.g., create a function like normalizeEngramContent(content: string): string) and replace the inline code in before-compaction.ts (currently calling stripEngramContext and slicing with CONTENT_MAX_CHARS) and the similar block in session-end.ts to call that helper; the helper should call stripEngramContext(content) then enforce the CONTENT_MAX_CHARS truncation consistently and be exported from a common module so both hooks import and use it.plugin/openclaw-engram/src/hooks/after-tool-call.ts (2)
59-62:PluginLoggerподключен, но фактически не используется в contradiction flow.Сейчас на Line 61 в
checkContradictions(...)передаетсяundefined, поэтому логирование уходит вconsole, а не в scoped plugin logger.Предлагаемое исправление
export function handleAfterToolCall( event: AfterToolCallEvent, ctx: PluginHookContext, client: EngramRestClient, config: PluginConfig, + logger?: PluginLogger, ): void { @@ if (isWriteOrEdit(event.toolName)) { - void checkContradictions(client, project, event.toolResult, undefined).catch(() => {}); + void checkContradictions(client, project, event.toolResult, logger).catch(() => {}); } }Also applies to: 80-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/hooks/after-tool-call.ts` around lines 59 - 62, The contradiction-checking path uses checkContradictions(client, project, event.toolResult, undefined) which passes undefined for the logger so it falls back to console instead of the scoped PluginLogger; update the call sites (e.g., inside the isWriteOrEdit branch and the similar block around lines 80-85) to pass the existing PluginLogger instance (the scoped plugin logger variable available in this module) instead of undefined, and ensure the promise rejection handler uses PluginLogger.error (or PluginLogger.{warn|info} as appropriate) so all contradiction flow logging goes through the plugin-scoped logger rather than console.
74-78: Проверку Write/Edit лучше нормализовать черезtoLowerCase().Текущий
Setс дублированием регистров работает, но проще и надежнее нормализоватьtoolNameодин раз.Предлагаемое упрощение
-const WRITE_TOOLS = new Set(['write', 'edit', 'Write', 'Edit']); +const WRITE_TOOLS = new Set(['write', 'edit']); function isWriteOrEdit(toolName?: string): boolean { - return toolName != null && WRITE_TOOLS.has(toolName); + return typeof toolName === 'string' && WRITE_TOOLS.has(toolName.toLowerCase()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/openclaw-engram/src/hooks/after-tool-call.ts` around lines 74 - 78, Normalize the write/edit check by lowercasing the tool name once: change WRITE_TOOLS to contain lowercase variants (e.g., 'write', 'edit') and update isWriteOrEdit to call toolName?.toLowerCase() before checking WRITE_TOOLS.has(...); reference WRITE_TOOLS, isWriteOrEdit, and the toolName parameter when making this change.internal/db/gorm/migrations.go (1)
1156-1157: Для аналитических запросов не хватает составного индекса поproject/query.Текущие индексы на Line 1156-1157 помогут фильтрации по проекту и времени, но для
GROUP BY queryпо проекту обычно эффективнее составной индекс(project, query, created_at DESC).Предлагаемое изменение миграции
`CREATE INDEX IF NOT EXISTS idx_search_misses_project ON search_misses (project)`, `CREATE INDEX IF NOT EXISTS idx_search_misses_created ON search_misses (created_at)`, + `CREATE INDEX IF NOT EXISTS idx_search_misses_project_query_created + ON search_misses (project, query, created_at DESC)`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/db/gorm/migrations.go` around lines 1156 - 1157, Добавить составной индекс для ускорения аналитики по проекту и запросу: вместо/в дополнение к существующим индексам на search_misses (idx_search_misses_project, idx_search_misses_created) создать SQL-миграцию, которая выполняет CREATE INDEX IF NOT EXISTS idx_search_misses_project_query_created_at ON search_misses (project, query, created_at DESC) (или аналогичное имя индекса), чтобы обеспечить эффективные GROUP BY/фильтрации по project и query с учётом порядка по created_at; внесите это изменение в тот же файл миграции (migrations.go), рядом с существующими индексными CREATE-выражениями для таблицы search_misses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/db/gorm/observation_store.go`:
- Around line 1429-1450: RecordSearchMiss currently inserts raw user queries
(and GetSearchMissStats returns them), which risks storing empty/very
long/PII-bearing strings; update RecordSearchMiss to normalize the query before
inserting by trimming whitespace, rejecting or not inserting empty strings,
enforcing a max length (e.g. truncate to N chars) and applying optional
redaction/normalization rules (e.g. remove sensitive tokens); ensure
GetSearchMissStats uses the same normalized/trimmed value when grouping (i.e.,
persist normalized_query or compute normalization before insert) so stats return
the cleaned value; reference the RecordSearchMiss and GetSearchMissStats logic
and the search_misses insert/SELECT to locate and apply the change.
In `@internal/privacy/secrets_test.go`:
- Around line 182-184: The fixture in the test table (the case with fields name:
"API key with prefix", input: "api_key=abc123...") contains a realistic-looking
secret; replace the hard-coded secret literal with a value built at runtime so
the pattern remains valid but no real-looking key is stored. In the test in
internal/privacy/secrets_test.go update the test case's input construction (the
table entry with fields name,input,expectedCount) to generate the value during
execution (e.g., build "api_key="+a generated or repeated-safe string of the
same shape/length using strings.Repeat or crypto/rand/hex) so expectedCount
remains 1 while the repository no longer contains a plausible secret.
In `@internal/privacy/secrets.go`:
- Around line 65-69: ExtractSecrets() currently uses secretPatterns that match
only the PEM BEGIN header, so only the header is extracted and the base64 body
remains; change handling so PEMs are matched as full multiline blocks from
"-----BEGIN ...-----" through the corresponding "-----END ...-----" (use a
DOTALL/ (?s) or explicit [\s\S] pattern) and remove the entire matched block
from the text. Update secretPatterns to include a dedicated pemPattern that
matches the whole block, and in the loop inside ExtractSecrets() specifically
detect pemPattern (or add a new branch) to extract and store the entire block
(or call a new extractPemBlock function) instead of calling extractSecretValue
which strips only the header; ensure the removed substring is the full PEM block
so no base64 body remains in the text sent to the LLM.
- Around line 74-82: Короткий префикс hashPrefix (8 hex символов) даёт только 32
бита и приводит к коллизиям/потере записей при дедупе; измените вычисление имени
в функции, где используются hash := sha256.Sum256([]byte(value)), hashPrefix и
name := "auto:"+hashPrefix, чтобы использовать более длинный идентификатор
(лучше весь SHA-256 hex — hex.EncodeToString(hash[:]) без усечения, либо явно
увеличить длину префикса до 64 hex), и убедитесь, что ключи в seen и создаваемые
DetectedSecret{Name: name, Value: value} используют новый полный/длинный хеш
(обновите соответствующие ссылки на seen и DetectedSecret).
In `@internal/worker/handlers_backfill.go`:
- Around line 418-419: Вставка секретных credential-обservation через общий
StoreObservation вызывает их удаление обычным cleanup в ObservationStore;
исправьте так: не сохраняйте credential через обычный
StoreObservation(vaultSessionID, project, obs, ...), а либо вызовите/реализуйте
специализированный метод (например StoreCredentialObservation или
StoreObservationWithType(project, obs, type="credential")) и используйте его
вместо StoreObservation, либо измените логику очистки в ObservationStore
(cleanup/archival) чтобы явно пропускать записи с type="credential" при
удалении; поправьте вызов с использованием new method/flag и обновите
ObservationStore cleanup условие, ссылаясь на vaultSessionID, StoreObservation и
type="credential".
In `@internal/worker/handlers_context.go`:
- Around line 344-347: Current code spawns a new goroutine for every search miss
(the go s.trackSearchMiss(...) call when clusteredObservations is empty), which
can create unbounded goroutines under load; instead add a dedicated buffered
channel (e.g., searchMissCh) on the receiver struct and a single background
worker goroutine that reads from it and calls s.trackSearchMiss(project, query).
Replace the inline go call with a non-blocking send to that channel (select with
default) so callers don't block when the buffer is full; ensure the background
worker is started once (e.g., in the struct initializer) and drains the channel,
and reference the trackSearchMiss method and the clusteredObservations/query
check when making this change.
- Around line 1075-1089: В блоке, где формируются параметры для decisions search
(используются body.Query, body.Project, body.Limit и search.SearchParams),
добавьте валидацию имени проекта через ValidateProjectName(body.Project) и
возвращайте http.StatusBadRequest с понятным сообщением при невалидном project;
также введите верхнюю границу для limit (например const maxLimit = 100) и
обрезайте/откатывайте body.Limit к этому maxLimit перед присвоением в
SearchParams, чтобы предотвратить чрезмерно тяжёлые запросы.
- Around line 1147-1161: Validate the incoming body.Project using
ValidateProjectName and return http.StatusBadRequest on failure, and enforce a
safe bound on body.Limit before calling obsStore.GetSearchMissStats: if
body.Limit is zero or negative set a sensible default (e.g. 100), and if it
exceeds a defined maximum (e.g. MaxSearchMissLimit or 1000) cap it to that max;
then pass the validated body.Project and the capped limit into
obsStore.GetSearchMissStats. Ensure you reference/introduce a constant like
MaxSearchMissLimit nearby and use the existing ValidateProjectName and
GetSearchMissStats calls so the change is localized to this analytics endpoint
handler.
- Around line 1127-1133: trackSearchMiss reads s.observationStore without
synchronization causing a data race with writers that set it under s.initMu; fix
by grabbing the store reference under the initMu read lock (use
s.initMu.RLock()/RUnlock()) or otherwise atomic/copying the pointer into a local
var while holding the lock, check nil on that local before proceeding, then
create the timeout context from s.ctx (context.WithTimeout(s.ctx,
5*time.Second)) instead of context.Background() and use the local store to call
RecordSearchMiss so the goroutine never races with concurrent reassignments.
In `@plugin/openclaw-engram/src/context/tiers.ts`:
- Around line 97-103: The code in tiers.ts uses JSON.stringify(m).length inside
the classify() flow which can throw on cyclic objects or BigInt and crash the
classifier; wrap the serialization in a safe fallback: attempt JSON.stringify(m)
in a try/catch (or use a safe stringify helper with a replacer that converts
BigInt to string and breaks cycles), and if serialization fails, fall back to a
non-throwing representation such as String(m) or util.inspect(m) and use its
length; update the branch that computes totalChars (the block handling m as
Record<string, unknown> and the else branch using JSON.stringify) to use this
safe serialization so classify() no longer throws on problematic inputs.
In `@plugin/openclaw-engram/src/hooks/before-prompt-build.ts`:
- Around line 67-70: The code is sending raw event.prompt to analytics
(client.trackSearchMiss) which can leak sensitive or very long text; modify
before-prompt-build.ts so you normalize and truncate/redact the prompt before
sending: implement or call a helper like sanitizeAnalyticsQuery(normalizePrompt)
to strip/normalize whitespace, remove PII/URLs/emails, and truncate to a safe
max length (e.g., 200 chars) and then pass that sanitized string as query to
trackSearchMiss({ project, query: sanitizedQuery }); keep the fire-and-forget
pattern and preserve the same call site (client.trackSearchMiss) and ensure
empty or fully-redacted prompts are not sent.
---
Outside diff comments:
In `@internal/worker/sdk/processor.go`:
- Around line 639-645: The loop in callLLM creates llmCtx from
context.Background(), ignoring the incoming ctx; change llmCtx creation to use
the caller's ctx (llmCtx, cancel := context.WithTimeout(ctx, 120*time.Second))
so parent cancellations propagate to LLM calls, and ensure cancel() is called
immediately after the p.llmClient.Complete call (do not use defer inside the
retry loop to avoid accumulating defers). Also check ctx.Err() between retries
and break/return early if the parent context is canceled; adjust references to
llmCtx, cancel, and the p.llmClient.Complete call accordingly.
---
Nitpick comments:
In `@internal/db/gorm/migrations.go`:
- Around line 1156-1157: Добавить составной индекс для ускорения аналитики по
проекту и запросу: вместо/в дополнение к существующим индексам на search_misses
(idx_search_misses_project, idx_search_misses_created) создать SQL-миграцию,
которая выполняет CREATE INDEX IF NOT EXISTS
idx_search_misses_project_query_created_at ON search_misses (project, query,
created_at DESC) (или аналогичное имя индекса), чтобы обеспечить эффективные
GROUP BY/фильтрации по project и query с учётом порядка по created_at; внесите
это изменение в тот же файл миграции (migrations.go), рядом с существующими
индексными CREATE-выражениями для таблицы search_misses.
In `@plugin/openclaw-engram/src/hooks/after-tool-call.ts`:
- Around line 59-62: The contradiction-checking path uses
checkContradictions(client, project, event.toolResult, undefined) which passes
undefined for the logger so it falls back to console instead of the scoped
PluginLogger; update the call sites (e.g., inside the isWriteOrEdit branch and
the similar block around lines 80-85) to pass the existing PluginLogger instance
(the scoped plugin logger variable available in this module) instead of
undefined, and ensure the promise rejection handler uses PluginLogger.error (or
PluginLogger.{warn|info} as appropriate) so all contradiction flow logging goes
through the plugin-scoped logger rather than console.
- Around line 74-78: Normalize the write/edit check by lowercasing the tool name
once: change WRITE_TOOLS to contain lowercase variants (e.g., 'write', 'edit')
and update isWriteOrEdit to call toolName?.toLowerCase() before checking
WRITE_TOOLS.has(...); reference WRITE_TOOLS, isWriteOrEdit, and the toolName
parameter when making this change.
In `@plugin/openclaw-engram/src/hooks/before-compaction.ts`:
- Around line 49-52: Extract the repeated strip-and-truncate logic into a shared
helper (e.g., create a function like normalizeEngramContent(content: string):
string) and replace the inline code in before-compaction.ts (currently calling
stripEngramContext and slicing with CONTENT_MAX_CHARS) and the similar block in
session-end.ts to call that helper; the helper should call
stripEngramContext(content) then enforce the CONTENT_MAX_CHARS truncation
consistently and be exported from a common module so both hooks import and use
it.
🪄 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: 46aa4a15-f006-4bef-a86e-3cfd42dc4308
📒 Files selected for processing (23)
internal/config/config.gointernal/config/config_test.gointernal/db/gorm/migrations.gointernal/db/gorm/observation_store.gointernal/graph/edge_detector.gointernal/privacy/secrets.gointernal/privacy/secrets_test.gointernal/vector/pgvector/client.gointernal/vector/types.gointernal/worker/handlers_backfill.gointernal/worker/handlers_context.gointernal/worker/sdk/processor.gointernal/worker/sdk/processor_test.gointernal/worker/service.goplugin/openclaw-engram/package.jsonplugin/openclaw-engram/src/client.tsplugin/openclaw-engram/src/context/tiers.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/security/redactor.tsplugin/openclaw-engram/src/types/openclaw.ts
💤 Files with no reviewable changes (4)
- internal/config/config.go
- internal/config/config_test.go
- internal/graph/edge_detector.go
- plugin/openclaw-engram/src/security/redactor.ts
Owner
Author
|
@codex review |
thebtf
added a commit
that referenced
this pull request
Mar 17, 2026
…raction, hash collision handlers_context.go: - CRIT: Fix data race in trackSearchMiss — read s.observationStore under s.initMu.RLock and use s.ctx instead of context.Background(). - MAJOR: Remove per-search-miss goroutine spawn — call trackSearchMiss inline to prevent unbounded goroutine growth under load. - MAJOR: Add ValidateProjectName + cap limit (max 100) in handleSearchDecisions. - MAJOR: Add ValidateProjectName + cap limit (max 200, default 50) in handleSearchMissAnalytics. internal/privacy/secrets.go: - CRIT: Add full multiline PEM block pattern (BEGIN...END) so ExtractSecrets captures the entire key material, not just the header line. Header-only pattern retained for ContainsSecrets detection. - MAJOR: Use full 64-char SHA-256 hex as auto: vault key name to eliminate 32-bit hash collision risk in deduplication logic. internal/privacy/secrets_test.go: - MINOR: Replace realistic-looking API key fixture with strings.Repeat to avoid secret scanner false positives (gitleaks high alert). internal/db/gorm/observation_store.go: - MAJOR: Normalize query in RecordSearchMiss — trim whitespace, reject empty queries after trim, retaining existing 500-char cap. - MAJOR: Exclude type='credential' from CleanupOldObservations so credentials are never silently evicted by the rotation policy. internal/db/gorm/migrations.go: - MAJOR: Add migration 034 — partial UNIQUE INDEX on (project, title) WHERE type='credential' to prevent duplicate credential entries from concurrent backfill races; also adds composite (project, query, created_at DESC) index on search_misses for efficient GROUP BY analytics. plugin/openclaw-engram/src/hooks/after-tool-call.ts: - N/A: Add optional logger param to handleAfterToolCall and pass it to checkContradictions so warnings route to agent's log stream, not console. plugin/openclaw-engram/src/hooks/before-prompt-build.ts: - MAJOR: Normalize and truncate raw prompt (trim, collapse whitespace, max 512 chars) before sending to trackSearchMiss analytics to reduce PII retention. plugin/openclaw-engram/src/context/tiers.ts: - MAJOR: Wrap JSON.stringify in try/catch in estimateContextFill to prevent crashes on cyclic objects or BigInt values.
#49) Configurable via env var ENGRAM_LLM_MAX_TOKENS (default: 4096). Stored in config.Config.LLMMaxTokens and OpenAIConfig.MaxTokens. Removes magic number from LLM client. Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
…_results (#50) - Add type=guidance to typeWeights (1.8, highest) and typeHalfLife (365 days) - Behavioral rules no longer decay in 7 days or get default weight 1.0 - sourceBoost 1.3 for LLM-extracted guidance (live user_behavior detection) - total_results now counts observations with composite score > 0.05 (was raw DB count — in high-dim space all observations passed threshold, showing "33 matches" for every query regardless of relevance) Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
Imported feedback rules (type=decision, concept=user-preference, title starts with "Rule:") were all classified as contradicting each other because classifyRelation marks any two decisions with different titles and similarity > 0.7 as contradicts. 57 rules × 56 peers = 76 false contradiction edges in the knowledge graph. Added hasGuidanceConcept() check: skips contradiction detection for observations that are behavioral rules (type=guidance, or concept user-preference, or title prefix "Rule:"). Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
Skip HEARTBEAT.md polling (openclaw every 30min) and Telegram conversation/sender metadata from being stored as user prompts. These are system-generated, not real user interactions. Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
PreCompact hook was created but never registered in hooks.json. Now registered with 10s timeout. Hook writes discovery data to .agent/pre-compact-discovery.json for empirical testing of available input fields (transcript_path verification for FR-2).
Three-tier injection system: observations tagged with concept "always-inject" are now fetched independently of similarity matching and included in every session (session-start) and every prompt (user-prompt) context. Server changes: - GetAlwaysInjectObservations query (concepts @> GIN index) - GIN indexes on concepts, files_modified, files_read columns - Migration 048 for all new indexes - handleContextInject + handleSearchByPrompt return always_inject array - AlwaysInjectLimit (default 20) and ProjectInjectLimit (default 15) config Hook changes: - session-start.js renders <user-behavior-rules> block before <engram-context> - user-prompt.js merges always-inject + similarity-matched rules with dedup - Plugin version bumped to 0.6.0 Also adds GetObservationsByFile and GetPreviousObservationInSession queries for Phase 3 and Phase 4 (no callers yet).
Reads transcript JSONL at compaction time, parses all user/assistant messages, and sends to /api/backfill/session in chunks of 50 messages. Fire-and-forget with 5s timeout per chunk (Constitution Principle 3). Fallback: if input.transcript_path is missing, derives path by searching ~/.claude/projects/<hash>/<session>.jsonl. Also writes discovery report to .agent/pre-compact-discovery.json for empirical verification of available hook input fields.
User got 400 trying to close issue #1 from dashboard — it was in 'reopened' state, not 'resolved'. CloseIssue was too strict. Now accepts: - resolved → closed: source confirms fix (normal flow) - reopened → closed: source decided issue no longer needed - Any state → closed: dashboard operator override Operator detection: source_project == 'dashboard'. Dashboard bypasses both state validation and source_project match checks. Bump all plugins to 3.5.3 (server parity).
Agent got 'project required' error without knowing full signature. Tool description now lists required params for EACH action upfront: create: action, project, title, target_project list: action get: action, id update: action, project, id, status=resolved comment: action, project, id, body reopen: action, project, id close: action, project, id Server validates all required params in one pass via validateIssueActionParams(), returning the full signature on error. Agents see what they forgot AND what else is needed. Bump plugins to 3.5.4.
…upfront Previously the JSON schema had a flat required=[action,project] list. Agents calling create() didn't see that title/target_project were also required until the server rejected the call. Blind-poke loop. Now the schema uses oneOf with 7 action-specific variants: create: required=[action, project, title, target_project] list: required=[action] get: required=[action, id] update: required=[action, project, id, status] comment: required=[action, project, id, body] reopen: required=[action, project, id] close: required=[action, project, id] Each variant has additionalProperties=false and uses const matching on the action field, so MCP clients that support oneOf see exactly the right params for the chosen action. Clients without oneOf support fall back to the flat properties list (still valid, just less strict). Bump plugins to 3.5.5.
Agent got 400 from Claude API: 'input_schema does not support oneOf, allOf, or anyOf at the top level' Root cause: two tools violated this constraint. 1. issues tool — my recent oneOf per-action schema 2. import_instincts (pre-existing) — anyOf for files/path mutual exclusion Fix: - issues: flat schema with REQUIRED_FOR/OPTIONAL_FOR prefixes in each parameter description. Per-action requirements still visible to agents, just in prose instead of JSON Schema. Server-side validation in validateIssueActionParams still enforces all rules with clear errors. - import_instincts: flat schema, mutual exclusion moved to server-side handler with explicit error messages. Added regression test: TestHandleToolsList now asserts NO tool has oneOf/allOf/anyOf at top level of input_schema. Caught both tools immediately — would have prevented this deployment accident. Bump plugins to 3.5.6.
Root cause: HTTP WriteTimeout=60s (set by PR #31 security hardening) closes idle SSE connections, EventSource fires onerror, dashboard shows banner, client reconnects — repeat every minute. Fix: SSE handler now sends a comment-line keepalive ': keepalive\n\n' every 25s. Comments are ignored by the client but keep the HTTP write path active, preventing WriteTimeout from firing on idle streams. Also added: - X-Accel-Buffering: no header to disable nginx/reverse-proxy buffering - Client.WriteMu mutex to serialize concurrent writes between broadcast goroutines and the keepalive ticker - Initial connection message now goes through the mutex too Verified: all SSE tests pass. Dashboard banner should stay hidden when connection is healthy. Bump plugins to 3.5.7.
…ching Three core problems addressed: 1. Agents were not noticing engram issues at session start. Root cause: hooks query by hashed project ID (e.g. mcp-mux_e54050) while issues are stored by bare slug (e.g. mcp-mux). The two namespaces never matched. Server now matches BOTH forms via projectBareName() helper — query "mcp-mux_e54050" matches target_project="mcp-mux" AND vice versa. 2. No structured workflow for issue triage. Agents had to be told to check issues every time. Added /engram:issue slash command that walks through: live issues, your reopens, your cross-project issues to verify, file new ones for tracked projects only. 3. Agents had no way to know which projects use engram issues. New endpoint GET /api/issues/tracked-projects returns the set of projects with observations or issues — meaning agents working on them will actually see injected context. For non-tracked projects, use GitHub/Linear/etc. Other improvements: - Open-issues injection block now declares 'action-required="true"' with explicit MUST language and points at /engram:issue - Resolved-issues block similarly demands verification action - MCP instructions point at /engram:issue and explain tracked-projects Bump all plugins to 3.6.0 (minor — new command + new endpoint).
thebtf
added a commit
that referenced
this pull request
Apr 10, 2026
CRITICAL: HOOKS_WITH_EVENT_NAME set was missing 'SessionStart'.
writeResponse() only emits hookSpecificOutput when the hook name is in
that set — for SessionStart it was falling through to bare {continue:true},
silently discarding all computed context:
- <open-issues> block with action-required issues
- <resolved-issues> block for source-agent verification
- <user-behavior-rules> always-inject rules
- <engram-context> with project memory
The hook was doing all the work — fetching issues, formatting blocks,
logging 'Injecting N open issues' — and then throwing the result away
at the output step. This is why agents never saw issues at session start,
only behavioral rules arrived (via the UserPromptSubmit hook which WAS
in the set) and only after the first user prompt.
Fix: add 'SessionStart' to HOOKS_WITH_EVENT_NAME.
Verified by running the hook directly against the live server with a
test issue (#9 self-targeted to engram project). Before fix: stdout was
{"continue":true}. After fix: stdout contains hookSpecificOutput with
the full open-issues block, behavioral rules, and engram context.
Bump plugins to 3.6.1.
CRITICAL: HOOKS_WITH_EVENT_NAME set was missing 'SessionStart'.
writeResponse() only emits hookSpecificOutput when the hook name is in
that set — for SessionStart it was falling through to bare {continue:true},
silently discarding all computed context:
- <open-issues> block with action-required issues
- <resolved-issues> block for source-agent verification
- <user-behavior-rules> always-inject rules
- <engram-context> with project memory
The hook was doing all the work — fetching issues, formatting blocks,
logging 'Injecting N open issues' — and then throwing the result away
at the output step. This is why agents never saw issues at session start,
only behavioral rules arrived (via the UserPromptSubmit hook which WAS
in the set) and only after the first user prompt.
Fix: add 'SessionStart' to HOOKS_WITH_EVENT_NAME.
Verified by running the hook directly against the live server with a
test issue (#9 self-targeted to engram project). Before fix: stdout was
{"continue":true}. After fix: stdout contains hookSpecificOutput with
the full open-issues block, behavioral rules, and engram context.
Bump plugins to 3.6.1.
…realtime feedback (#134) * fix(search): honor LLM filter empty set as silence [T008] When the LLM explicitly returns an empty JSON array (intentional silence), return []int64{} instead of falling back to composite scoring top-5. The error/timeout fail-open path (returns allIDs) is preserved for resilience. Adds 3 unit tests: empty-set silence, parse-error fallback, timeout fallback. * fix(handlers): propagate LLM silence to inject response [T009] When FilterByRelevance returns an empty slice (LLM silence gate), set clusteredObservations to empty instead of silently keeping the pre-filter set. Adds Info-level log "LLM filter silenced injection" with project and total_considered fields for operator observability (NFR-5). * fix(search): honor LLM filter empty set as silence [T008] * fix(config): default injection floor to 0 [T006] Disables the injection floor anti-pattern by defaulting InjectionFloor to 0. With floor=0 the silence path is active: no fill observations are injected when composite scoring eliminates all candidates (FR-1, learning-memory-v4). Operators can restore legacy behavior via ENGRAM_INJECTION_FLOOR=3. * fix(handlers): skip floor fill when floor is 0 [T007] Removes the hardcoded fallback that forced injectionFloor=3 even when the config set it to 0. Both floor-fill sites in handlers_context.go are now guarded by `if injectionFloor > 0` so the silence path (FR-1) is honoured. Extracts fillToFloor helper (floor_fill.go) used by the search path site; the inject path site keeps inline logic to correctly account for the total across all four injection sections. Unit tests in handlers_context_floor_test.go cover floor=0 no-fetch, floor=3 fill, already-met no-fetch, and deduplication. Anti-stub check confirmed: stubbing fillToFloor body fails the fill test. * fix(search): honor LLM filter empty set as silence [T008] * fix(handlers): propagate LLM silence to inject response [T009] * feat(db): add utility_propagated_at column migration [T014] * feat(api): POST /api/sessions/{id}/propagate-outcome endpoint [T015] * feat(plugin): register SessionEnd hook (v3.7.0) [T016] * feat(plugin): session-end.js fires propagate-outcome [T017] * fix(maintenance): skip recently-propagated sessions [T018] * refactor(worker): extract RetrieveRelevant shared retrieval function [T010] * fix(mcp): set_session_outcome triggers PropagateOutcome [T019] Wire injectionStore into MCP Server and add PropagateOutcome goroutine to handleSetSessionOutcomeMCP, matching the pattern in handlers_learning.go. This closes the bypass where MCP tool callers would update session outcome without triggering utility score propagation. * fix(handlers): replace hardcoded inject query with RetrieveRelevant [T011] * feat(config): ENGRAM_INJECT_UNIFIED rollback flag (default true) [T012] * chore(bench): inject latency benchmark script [T013] * docs(changelog): add [3.7.0] learning-memory-v4 MVP entry * fix(handlers): atomic TOCTOU-free rate limit in propagate-outcome [pr134-H1+H2] Replace the read-then-check-then-write pattern in handlePropagateOutcome with an atomic SQL UPDATE ... WHERE utility_propagated_at IS NULL OR < NOW()-1min. Zero rows affected means rate-limited (HTTP 409). The goroutine now runs with a 30s context timeout and reverts the timestamp on failure so callers can retry. Response changed to HTTP 202 Accepted since propagation is dispatched async. Also resolves M3 (misnamed "updated" field removed) and pr134-major4 (timeout). New session_store methods: UpdateUtilityPropagatedAtIfStale, ClearUtilityPropagatedAt. Also fixes L2: UpdateUtilityPropagatedAt now uses time.Now().UTC(). * fix(mcp): add 30s timeout and update utility_propagated_at in set_session_outcome [pr134-major5+major6] The MCP set_session_outcome goroutine had two bugs: 1. context.Background() with no deadline -> goroutine leak on DB hang [pr134-major5] 2. Never updated utility_propagated_at after propagation -> maintenance guard does not see the propagation and double-runs [pr134-major6] Fix: wrap with context.WithTimeout(30s), and on success call UpdateUtilityPropagatedAt. * fix(bench): guard against empty HTTP_CODE on curl failure [pr134-M1] * fix(models): custom MarshalJSON for Session.UtilityPropagatedAt [pr134-major1] sql.NullTime serializes as {"Time":"...","Valid":true} which leaks the DB implementation detail into the public API JSON. Add SDKSession.MarshalJSON() using a shadow struct that emits utility_propagated_at as an RFC3339 string when set, or omits it when null. * docs(plugin): explain SessionEnd hook timeout budget [pr134-L1] * feat(db): add partial index on sdk_sessions.utility_propagated_at [pr134-minor-idx] * test(config): use s.T().Setenv() for automatic cleanup [pr134-minor-setenv] * docs(changelog): fix mojibake, enforce ASCII-only content [pr134-minor-changelog-ascii] * docs(changelog): add [3.7.0] reference link [pr134-minor-changelog-link] * docs(changelog): remove future-work section from [3.7.0] entry [pr134-gemini-changelog] * fix(retrieval): inject query derived from session_id, not project-wide [pr134-major2] Add loadLastUserPromptBySession which scopes the inject-query derivation to the current claude_session_id. Previously loadRecentUserPromptsByProject returned the latest prompt across ANY session in the project, so session A could be seeded by session B's last prompt — breaking the feature promise. New fallback chain: session_id != "" → last prompt for that session (via getLastPromptBySession hook) session_id == "" → last prompt for the project (cold-start) both empty → project name as literal query (ultimate fallback) Add getLastPromptBySession hook to retrievalHooks for test injection. Update handlers_context.go doc comment to reflect session-scoped semantics. * test(inject): exercise real HTTP handler with seeded sessions and prompts [pr134-major7] Rewrite inject unified tests to use the new session-scoped lookup hook (getLastPromptBySession) instead of the old project-wide hook. Add: - TestInjectRelevant_UnifiedPath_UsesLastUserPrompt: verifies that only the correct session's prompt is used as the retrieval query. - TestInjectRelevant_TwoSessionsDifferentPrompts: anti-stub test — two sessions with different prompts must produce different queries. Replacing the session-scoped hook with project-wide fallback makes both sessions return the project name as query (q1 == q2), failing this test. - TestInjectRelevant_SessionScoped_IgnoresOtherSessionPrompts: verifies session-auth and session-db produce completely different inject queries. - TestInjectRelevant_LegacyPath_WhenFlagFalse: unchanged, verifies that InjectUnified=false skips the unified path entirely. * refactor(retrieval): simplify complex condition [pr134-minor1] The applyReranking early-return condition combined three nil-checks with mixed && / || precedence into a single hard-to-parse line. Extract to a named boolean noRerankAvailable and make the && grouping explicit, matching the original semantics exactly but readable at a glance. * docs(retrieval): document NoiseFloorScore 0.05 threshold rationale [pr134-minor2] Extract the 0.05 literal to a named const NoiseFloorScore with a doc comment explaining why the threshold exists: in high-dimensional embedding spaces nearly all observations pass the raw vector threshold, so only composite scores above 0.05 represent genuine semantic matches. * docs(retrieval): document 0.9 magic number [pr134-minor3] Two separate 0.9 constants in retrieval.go now have names and rationale: - vectorPreFilterFactor (0.9): widens the vector pre-filter by 10% below the project threshold, giving borderline-relevant observations a chance to reach the reranker before being discarded. - clusteringThreshold default (0.9): cosine similarity at which two observations are considered near-duplicates and merged into one cluster. Documented with an inline comment; overridable via config. * docs(handlers): fix mojibake in handlers_context.go comments [pr134-gemini1] Eight comment strings contained CP1251-encoded text (—) that was stored as if it were UTF-8, producing unreadable mojibake. Replace all occurrences with the correct UTF-8 em dash (—). Lines affected: 290, 692, 710, 762, 801, 1021, 1221, 1370. --------- Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
* fix: add store-path supersession kill-switch * feat(vector): add file-path filters to BuildWhereFilter [T021] * feat(hooks): track edited files in session signal store [T022] * feat(hooks): pass files_being_edited in inject and search hooks [T023] * feat(worker): apply file-scope filters in unified retrieval path [T024] * feat(search): add SearchLaneConfig defaults [T025] * feat(config): add typed lane config wiring [T026] * feat(worker): complete learning-memory-v4 follow-up phases Ship the post-MVP retrieval, briefing, trigger, merge, graph, and dashboard wave so GitHub can build and publish the new release for live validation. * fix: address PR #135 review findings across merge, lanes, and read triggers Fix write-merge action handling for SUPERSEDE/UPDATE, preserve typed-lane MinScore semantics under graph fusion, and make repeated-read triggers work in remote worker setups by passing read-count signals in trigger requests. * fix: harden trigger isolation and merge decision validation Scope read-trigger file lookups by project, strip sensitive payloads from Edit/Write trigger queries, normalize trigger request params in pre-tool-use hook, and reject UPDATE/SUPERSEDE merge decisions without target IDs. * fix(pr-135): close major reviewer findings in retrieval safety and sanitization Addresses unresolved review threads for lane config env override, command-prefix escaping, commands_run redaction, trigger query hardening, and test isolation improvements. * fix(pr-135): address remaining reviewer findings Resolve remaining CodeRabbit findings by reducing test coupling, hardening MCP response parsing, adding debug logging for entity observation fetch failures, escaping XML in test payload construction, and aligning wording/UI labels with snapshot behavior. Reviewer comment ids: coderabbit-nitpick-c839c8da-39, coderabbit-nitpick-55d37369-370, coderabbit-nitpick-d5dafde0-230, coderabbit-nitpick-6c36cc5c-1889, PRRC_kwDORY-ek8625tfQ, PRRC_kwDORY-ek8625tfS Thread ids: coderabbit-nitpick-c839c8da-39, coderabbit-nitpick-55d37369-370, coderabbit-nitpick-d5dafde0-230, coderabbit-nitpick-6c36cc5c-1889, PRRT_kwDORY-ek856Uw-D, PRRT_kwDORY-ek856Uw-F * fix(pr-135): close coderabbit findings in merge, retrieval, and hooks Address all currently unresolved CodeRabbit threads by hardening write-merge consistency, normalizing retrieval seed/neighbor behavior, and fixing hook trigger/path edge cases with targeted tests. Reviewer comment ids: coderabbit-nitpick-d52599af-1828, coderabbit-nitpick-4d0e6e77-2117, PRRC_kwDORY-ek86256dL, PRRC_kwDORY-ek86256dP, PRRC_kwDORY-ek86256dS, PRRC_kwDORY-ek86256dT, PRRC_kwDORY-ek86257x9, PRRC_kwDORY-ek86257x_, PRRC_kwDORY-ek86257yB Thread ids: coderabbit-nitpick-d52599af-1828, coderabbit-nitpick-4d0e6e77-2117, PRRT_kwDORY-ek856U7gL, PRRT_kwDORY-ek856U7gP, PRRT_kwDORY-ek856U7gS, PRRT_kwDORY-ek856U7gT, PRRT_kwDORY-ek856U8jS, PRRT_kwDORY-ek856U8jU, PRRT_kwDORY-ek856U8jW --------- Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
Restore the observations.commands_run upgrade path so live databases created before the new field can store sanitized command arrays without SQLSTATE 42703 failures.
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
Retrospective review PR covering two sessions of work (v0.4.0 → v0.5.0).
Features
Fixes
Refactoring
Migration
Test plan
go build ./cmd/worker/go test ./internal/privacy/... ./internal/worker/... ./internal/crypto/...npx tsc --noEmit(plugin)Summary by CodeRabbit
Release Notes
New Features
Changes
Chores