fix: isOwnedByAgent阻斷derived被main fallback錯誤繼承(#448)#509
fix: isOwnedByAgent阻斷derived被main fallback錯誤繼承(#448)#509jlin53882 wants to merge 13 commits intoCortexReach:masterfrom
Conversation
- Add _initialized singleton flag to prevent re-initialization when register() is called multiple times during gateway boot - Add per-entry debug logging for governance filter decisions (id, reason, score, text snippet) for observability - Export _resetInitialized() for test harness reset - Fixes initialization block repeated N times on startup - Fixes governance filter decisions not observable in logs
…ckground agents Background agents (e.g. memory-distiller, cron workers) that receive structured task prompts can have their output quality degraded when memory context is injected alongside raw input. autoRecallExcludeAgents allows per-agent opt-out at the injection layer. Adapted from PR CortexReach#307 (fix/autorecall-exclude-background-agents). Closes CortexReach#137.
Add `memory-pro import-markdown` command to migrate existing Markdown memories (MEMORY.md, memory/YYYY-MM-DD.md) into the plugin LanceDB store for semantic recall. This addresses Issue CortexReach#344 by providing a migration path from the Markdown layer to the plugin memory layer.
Add two improvements addressing Issue CortexReach#344: 1. README: add Dual-Memory Architecture section explaining Plugin Memory (LanceDB) vs Markdown Memory distinction 2. index.ts: log dual-memory warning on plugin startup Refs: CortexReach#344
…in finally Reviewer: rwmjhb (CortexReach) 1. Schema completeness: add rerankTimeoutMs to openclaw.plugin.json retrieval schema and index.ts PluginConfig.retrieval 2. Timeout cleanup: move clearTimeout into finally block so timer is always cleared even on fast failure paths 3. Also update warning message to show actual configured timeout value Closes CortexReach#346
Files were deleted from origin/master during earlier rebase/cherry-pick operations. These deletions were unintentional — the PRs were meant to add features, not delete existing functionality. Restored from upstream/master: - src/: admission-control, admission-stats, auto-capture-cleanup, batch-dedup, clawteam-scope, identity-addressing, intent-analyzer, llm-oauth, memory-compactor, preference-slots, retrieval-stats, retrieval-trace, session-compressor, workspace-boundary - scripts/: governance-maintenance.mjs, migrate-governance-metadata.mjs - test/: 25 test files restored - README translations: DE, ES, FR, IT, JA, KO, PT-BR, RU, TW
# Conflicts: # scripts/governance-maintenance.mjs # scripts/migrate-governance-metadata.mjs
# Conflicts: # cli.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b7543ef63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
index.ts
Outdated
| if (_initialized) { | ||
| api.logger.debug("memory-lancedb-pro: register() called again — skipping re-init (idempotent)"); | ||
| return; | ||
| } | ||
| _initialized = true; |
There was a problem hiding this comment.
This module-level guard short-circuits every subsequent register() call in the same process, even when a new API instance is passed, and it sets _initialized before configuration/setup succeeds. In any re-registration flow (including existing test patterns that create multiple harnesses in one process), the later plugin instance will skip hook/tool registration entirely; if the first registration throws, all later attempts are blocked until restart. Move the flag assignment after successful setup or scope idempotency to the specific API/lifecycle instance.
Useful? React with 👍 / 👎.
cli.ts
Outdated
| } | ||
|
|
||
| try { | ||
| const vector = await context.embedder!.embedQuery(text); |
There was a problem hiding this comment.
The new import-markdown path embeds persisted memory text with embedQuery, while other ingestion paths store memories with embedPassage. With task-aware embedding configs (different query vs passage tasks), this writes imported entries into the wrong embedding space and makes their recall quality inconsistent or degraded compared with normal stored memories. This path should use passage embeddings for documents being stored.
Useful? React with 👍 / 👎.
AliceLJY
left a comment
AliceLJY
left a comment
There was a problem hiding this comment.
Thanks for the work on the isOwnedByAgent fix -- the core logic (gating derived items to their owning agent) is sound and well-reasoned.
However, this PR has significant scope and quality issues that need to be addressed before it can be merged:
1. Scope creep -- one PR should = one concern
This PR bundles at least 6 unrelated changes beyond the #448 fix:
import-markdownCLI command (new feature)autoRecallExcludeAgentsconfig option (new feature)rerankTimeoutMsconfig option (new feature)- Idempotent
register()guard (new behavior) - README dual-memory architecture rewrite (docs)
recallModeparsing fix (bug fix)
Each of these deserves its own PR. When unrelated changes are bundled, reviewers cannot approve the good parts without accepting the risky parts, and a bug in one area blocks the entire PR.
2. _initialized guard has a P1 bug
The flag is set before register() completes. If setup throws partway through, the plugin becomes permanently broken until process restart. The flag should be set at the end of successful registration.
3. embedQuery vs embedPassage in import-markdown
embedQuery() is for search queries, not documents being stored. This would put imported entries in the wrong embedding space.
Contribution workflow reminder
To help keep reviews efficient, here is the workflow we would like all contributors to follow:
When to open an Issue first (before writing code):
| Situation | Why |
|---|---|
| New feature or enhancement | Approach needs alignment before code is written |
| Bug affecting core behavior | Maintainers may know the root cause already |
| Design question | Implementation depends on the answer |
| Change touching multiple files/subsystems | Scope needs agreement upfront |
When you can PR directly:
- One-line fix with obvious correctness
- Bug you reported in an Issue AND got maintainer green light
- Test-only or documentation fixes
The workflow:
- Open Issue -- describe the problem and proposed approach
- Wait for feedback -- maintainer may approve, suggest changes, or say "not now"
- Get green light -- maintainer says "please go ahead with a PR"
- Submit ONE focused PR -- tested locally, one concern per PR
- Iterate in place -- push follow-up commits to the same branch, do not close and reopen as v2/v3/v4
Please split this PR into individual PRs (one per feature/fix), each with its own Issue for discussion. The isOwnedByAgent fix itself can stay -- just remove the unrelated changes.
|
好的，我在提交PR 時會保持一個問題+isses,謝謝你
安闲静雅 ***@***.***> 於 2026年4月4日 週六 下午7:55 寫道：
… ***@***.**** requested changes on this pull request.
Thanks for the work on the isOwnedByAgent fix -- the core logic (gating
derived items to their owning agent) is sound and well-reasoned.
However, this PR has significant scope and quality issues that need to be
addressed before it can be merged:
1. Scope creep -- one PR should = one concern
This PR bundles at least 6 unrelated changes beyond the #448
<#448> fix:
- import-markdown CLI command (new feature)
- autoRecallExcludeAgents config option (new feature)
- rerankTimeoutMs config option (new feature)
- Idempotent register() guard (new behavior)
- README dual-memory architecture rewrite (docs)
- recallMode parsing fix (bug fix)
Each of these deserves its own PR. When unrelated changes are bundled,
reviewers cannot approve the good parts without accepting the risky parts,
and a bug in one area blocks the entire PR.
2. _initialized guard has a P1 bug
The flag is set *before* register() completes. If setup throws partway
through, the plugin becomes permanently broken until process restart. The
flag should be set at the *end* of successful registration.
3. embedQuery vs embedPassage in import-markdown
embedQuery() is for search queries, not documents being stored. This
would put imported entries in the wrong embedding space.
------------------------------
Contribution workflow reminder
To help keep reviews efficient, here is the workflow we would like all
contributors to follow:
*When to open an Issue first (before writing code):*
Situation Why
New feature or enhancement Approach needs alignment before code is written
Bug affecting core behavior Maintainers may know the root cause already
Design question Implementation depends on the answer
Change touching multiple files/subsystems Scope needs agreement upfront
*When you can PR directly:*
- One-line fix with obvious correctness
- Bug you reported in an Issue AND got maintainer green light
- Test-only or documentation fixes
*The workflow:*
1. Open Issue -- describe the problem and proposed approach
2. Wait for feedback -- maintainer may approve, suggest changes, or
say "not now"
3. Get green light -- maintainer says "please go ahead with a PR"
4. Submit ONE focused PR -- tested locally, one concern per PR
5. Iterate in place -- push follow-up commits to the same branch, do
not close and reopen as v2/v3/v4
*Please split this PR* into individual PRs (one per feature/fix), each
with its own Issue for discussion. The isOwnedByAgent fix itself can stay
-- just remove the unrelated changes.
—
Reply to this email directly, view it on GitHub
<#509 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASID5C2LXIPNWJJYR4X5MET4UDZ2VAVCNFSM6AAAAACXMQEVCKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DANJYGQ2TKMRRGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Thanks for the reply! To be clear, there are concrete action items for this PR specifically: What you need to do now:
For the removed features, open an Issue for each one first, then PR after we discuss. Does that make sense? |
|
One more clarification on the workflow: An Issue is a proposal, not a notification. The process is:
Please do NOT open an Issue and a PR at the same time. The Issue is where we decide whether the change is needed and how it should be done. This avoids wasted effort on both sides. Also — small improvements can often be grouped. If you find 3 small bugs in the same file, one Issue + one PR covering all three is better than 3 separate Issues + 3 separate PRs. |
Review Claw 🦞 — 稽核修復已套用處理的 Bug（依維護者要求）1. _initialized flag 修復 2. embedQuery → embedPassage Commit\4ab267a\ — fix: move _initialized flag to end of register(); use embedPassage in import-markdown |
jlin53882
force-pushed
the
fix/issue-448-derived-ownership
branch
2 times, most recently
from
e4b162f to
4ab267a
Compare
… _initialized to end of register(); use embedPassage in import-markdown
jlin53882
force-pushed
the
fix/issue-448-derived-ownership
branch
from
4ab267a to
413133e
Compare
Review Claw 🦞 — PR 已精簡，只保留 #448 核心 fixScope Creep 項目已全數移除依 AliceLJY 要求，以下 scope creep 內容已從本 PR 移除：
最終 Commit\413133e\ — 精準三項修正：
總計：3 files changed, +16/-2 lines — 乾淨隔離，無 scope drift。 請重新 review。 |
|
本 PR 將關閉。Scope creep 問題已全部移除，乾淨版本將從 \jlin53882:fix/issue-448-v2\ branch 重新開 PR。感謝 AliceLJY 的 review！ |
PR #509 已關閉 — 原因說明本 PR（#509）已關閉，理由如下： Scope Creep 問題AliceLJY 的 review 指出，本 PR 混入了 6 個與 #448 fix 無關的變更：
處理方式不拆分，直接重新開 PR。 感謝 AliceLJY 的建議，我們 rebase 到 upstream master，只保留 #448 的核心修正。 後續處理內容：
如需個別 features（import-markdown、autoRecallExcludeAgents 等），請各自獨立開 Issue → PR。 Review Claw 🦞 — 協助清理與重新提交 |
2 participants
問題（Issue #448）
isOwnedByAgent()在src/reflection-store.ts將owner === "main"寫死為 fallback，導致所有子 agent 都會錯誤繼承 main agent 的derived類型 reflection lines（近期對話變化），造成 context bleed。修復內容
修改
isOwnedByAgent（src/reflection-store.ts:429）：itemKind === "derived"時，不做owner === "main"fallback，確保 derived 只對其擁有者可見itemKind === "derived"且owner為空白時，回傳false（空白 owner 的 derived 完全不可見，防止直接寫入 LanceDB 繞過 API 的洩漏風險）invariant/legacy/mapped類型維持原本的owner === "main"fallback，向後相容行為對照
測試驗證
migrate-legacy-schema.test.mjs因環境缺少proper-lockfile失敗，與本修改無關）技術債說明
injectMode: ["inheritance-only", "inheritance+derived"]在 config schema 有定義但程式碼未實作，屬獨立 feature request，與本 fix 正交