fix: skip auto-recall in the reflection distiller sub-session#922
fix: skip auto-recall in the reflection distiller sub-session#922gorkem2020 wants to merge 1 commit into
Conversation
gorkem2020
force-pushed
the
fix/reflection-distiller-hook-skip
branch
from
6809d84 to
e1cc8be
Compare
|
This branch now conflicts with the latest Please rebase onto or merge the latest |
…ession The reflection distiller runs its own embedded sub-session (sessionKey shaped temp:memory-reflection:<agentId>) to summarize the transcript being reflected on. The auto-recall before_prompt_build hook only skipped :subagent: sessions, so it still ran a full embedding + vector search and injected a recall block into the distiller's own prompt, contaminating the distillate with unrelated recalled memories instead of just the session content being summarized. The inherited-rules and derived-focus reflection injectors already guard against this sessionKey shape via isInternalReflectionSessionKey(). Apply the same guard to the auto-recall hook so all three injection paths agree: the distiller sub-session never receives injected context. Wire the new test file into the local npm test chain and the core-regression CI group.
gorkem2020
force-pushed
the
fix/reflection-distiller-hook-skip
branch
from
e1cc8be to
95b718a
Compare
|
Rebased onto latest master; the conflict is cleared and the PR is mergeable again. No content changes beyond the rebase. |
app3apps
left a comment
app3apps
left a comment
There was a problem hiding this comment.
Reviewed head 95b718a. The embedded reflection-distiller session is now excluded from auto-recall before embedding/retrieval, the focused and full suites pass, and I found no blocking issue.
Non-blocking follow-up: the supported CLI reflection fallback uses a different memory-reflection-cli-* identifier and is not marked as an internal reflection session, so the same contamination/cost issue can still occur on that pre-existing fallback path. Please address that in a focused follow-up with a fallback-path regression test.
The branch is one commit behind current master, but the orchestrator's trial merge was conflict-free and the relevant tests passed on the merged tree.
2 participants
What Problem This Solves
The plugin periodically runs a reflection distiller, an internal sub-session that reads a finished conversation and summarizes it into a short set of invariants and derived focus notes. This sub-session has its own session key, shaped like temp:memory-reflection:agentId, separate from the real user session it is summarizing.
The auto-recall hook that injects previously stored memories into a prompt only checked for sub-agent sessions. It did not recognize the distiller's own session key shape, so every time the distiller ran, auto-recall still fired inside it: a full embedding call and vector search happened, and a block of unrelated recalled memories was injected into the distiller's own prompt.
Why This Change Was Made
The reflection injectors that already exist for inherited rules and derived focus already skip this exact session key shape, using a helper that recognizes it. Auto-recall was the one place that had not been updated to use the same check, so the distiller's summarization pass was being contaminated with content that has nothing to do with the conversation it is supposed to be distilling. This change closes that one remaining gap using the same existing check already relied on elsewhere in the file, so all three prompt-injection paths now agree on this session key shape.
User Impact
After this change, when the distiller runs, it sees only the conversation it is meant to summarize, not injected recall content. The distilled invariants and derived focus notes should track actual session content more closely, since they are no longer competing with an unrelated memory block. There is no effect on any other session type: normal user sessions and sub-agent sessions behave exactly as before, and the existing auto-recall behavior for those is covered by a regression test.
Evidence
Added a new test file exercising the auto-recall hook directly with the distiller's session key shape, confirming no retrieval happens and no context is injected.
Included a regression test confirming a normal control session still reaches the retriever as before.
Included two additional tests confirming the existing inherited-rules and derived-focus injectors already correctly skip this session key shape, documenting that those paths were already safe.
Verified the new failing assertion is red against the code before this change (the auto-recall hook ran the retriever twice for the distiller's session key) and green after the fix.
Ran the full local test suite; all tests pass (one unrelated test that binds to a fixed local port was skipped due to a pre-existing environment conflict with an already-running local service, unrelated to this change).
tsc build completes with no errors, and the compiled dist file reflects the same change.
Production verification on a live deployment: with auto recall enabled, a session reset fired the reflection distiller and the distillation completed with zero recall activity inside its sub-session (no embedding, no vector search, no injected block), while writing its reflection normally. On the pre-fix build the same sequence runs a full recall inside the distiller.