fix: block main derived bleed via legacy fallback in buildDerivedCandidates#710
fix: block main derived bleed via legacy fallback in buildDerivedCandidates#710jlin53882 wants to merge 5 commits intoCortexReach:masterfrom
Conversation
…tate Resolves reviewer Must-Fix #1 from PR CortexReach#522: - _registeredApis.add(api) was called BEFORE _initPluginState. If _initPluginState threw, the api was already in the WeakSet, permanently blocking any retry from the same api instance. - Fix: wrap _initPluginState in try-catch; add api to WeakSet only after successful init. If init fails, api stays out of WeakSet, allowing subsequent register() calls with the same api to retry. Fixes CortexReach#448
- Issue 1: split 'if' from comment line + add missing fallback block - Issue 2: test now imports from ../src/reflection-store.ts (no local copy) - Issue 3: normalize all CR to LF (UTF-8 + LF) Fixes reviewer Must-Fix from PR CortexReach#522
…idates Layer 1 of 2 — blocks the read-path leak without changing write behavior. Problem: buildDerivedCandidates() falls back to legacyRows when itemCandidates is empty. combined-legacy rows (written by buildLegacyCombinedPayload) carry metadata.derived but have no itemKind, so isOwnedByAgent accepts owner==='main' for any sub-agent — causing context bleed. Fix: before the legacy flatMap, filter out rows where: - derived content exists (hasDerivedContent) - AND owner is 'main' (owner === 'main' → reject) - AND owner is empty (empty owner with derived → reject) - AND owner does not match querying agentId Pure legacy invariants (no derived) are unaffected. Added: buildDerivedCandidates-legacy-fallback.test.mjs with 7 cases covering all fallback paths including the main→sub-agent bleed (case A). Review: CortexReach#522 (review)
![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: b02f15c080
ℹ️ 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".
| if (!hasDerivedContent) return true; // 無 derived → 正常 legacy invariant | ||
| const owner = typeof metadata.agentId === "string" ? metadata.agentId.trim() : ""; | ||
| if (!owner) return false; // 有 derived 但無 owner → 不可見 | ||
| if (owner === "main") return false; // ★ main 的 derived 不外流 |
There was a problem hiding this comment.
In buildDerivedCandidates, the new legacy fallback filter unconditionally drops rows where owner === "main", so the main agent also loses access to its own legacy-derived reflections when itemCandidates is empty (the exact migration/backfill path this fallback is for). This regresses behavior for older memory-reflection records: a main-owned legacy row now contributes zero derived candidates even when agentId is "main".
Useful? React with 👍 / 👎.
1 participant
Summary
Layer 1 of 2 — blocks the read-path leak without changing write behavior.
Problem (reviewer feedback):
buildDerivedCandidates()falls back to legacyRows when itemCandidates is empty. The combined-legacy rows written bybuildLegacyCombinedPayloadcarrymetadata.derivedbut have noitemKind, soisOwnedByAgentacceptsowner==='main'for any sub-agent — causing context bleed.Fix: Before the legacy flatMap, filter out rows where derived content exists AND owner is 'main'.
Files changed:
src/reflection-store.ts—buildDerivedCandidatesnow filtersowner==='main'derived legacy rows in fallbacktest/buildDerivedCandidates-legacy-fallback.test.mjs— 7 regression cases covering all fallback pathsTesting
7 test cases covering:
Review reference
#522 (review)