fix(memory-core): prevent dreaming-narrative session leaks (#66358)#67023
fix(memory-core): prevent dreaming-narrative session leaks (#66358)#67023jalehman merged 6 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes the root cause of
Confidence Score: 3/5The primary timestamp-normalization fix is correct, but the two new defensive cleanup calls use the wrong session key format and will never delete anything. Two P1 findings: the safety-net deleteSession calls in runDreamingSweepPhases omit the workspace hash, so they construct keys that don't match those created by buildNarrativeSessionKey. The defensive cleanup is entirely non-functional. This doesn't reintroduce the original leak (the primary finally-block cleanup in generateAndAppendDreamNarrative is correct), but it means the stated 'defensive cleanup' goal of the PR is unmet. extensions/memory-core/src/dreaming-phases.ts — lines 1645 and 1669 (wrong session key format in defensive cleanup) Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/memory-core/src/dreaming-phases.ts
Line: 1645-1650
Comment:
**Defensive cleanup uses wrong session key format**
The defensive `deleteSession` calls construct keys like `dreaming-narrative-light-${sweepNowMs}`, but `buildNarrativeSessionKey` produces keys in the format `dreaming-narrative-${phase}-${workspaceHash}-${nowMs}`. The workspace hash segment is absent, so these calls always target a non-existent key — the safety net silently does nothing. The same mismatch exists for the REM cleanup at line 1669.
To fix, export `buildNarrativeSessionKey` from `dreaming-narrative.ts` (or re-inline the hash logic here) so the defensive cleanup constructs a key that actually matches what was created.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/memory-core/src/dreaming-phases.ts
Line: 1669-1674
Comment:
**Same wrong key format for REM cleanup**
`dreaming-narrative-rem-${sweepNowMs}` is missing the workspace hash, so this `deleteSession` call targets a non-existent key and the cleanup is a no-op, identical to the issue on the light-phase cleanup above.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(memory-core): prevent dreaming-narra..." | Re-trigger Greptile |
| const lightSessionKey = `dreaming-narrative-light-${sweepNowMs}`; | ||
| await params.subagent | ||
| .deleteSession({ sessionKey: lightSessionKey }) | ||
| .catch(() => { | ||
| // Swallow errors — this is best-effort cleanup. | ||
| }); |
There was a problem hiding this comment.
Defensive cleanup uses wrong session key format
The defensive deleteSession calls construct keys like dreaming-narrative-light-${sweepNowMs}, but buildNarrativeSessionKey produces keys in the format dreaming-narrative-${phase}-${workspaceHash}-${nowMs}. The workspace hash segment is absent, so these calls always target a non-existent key — the safety net silently does nothing. The same mismatch exists for the REM cleanup at line 1669.
To fix, export buildNarrativeSessionKey from dreaming-narrative.ts (or re-inline the hash logic here) so the defensive cleanup constructs a key that actually matches what was created.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/memory-core/src/dreaming-phases.ts
Line: 1645-1650
Comment:
**Defensive cleanup uses wrong session key format**
The defensive `deleteSession` calls construct keys like `dreaming-narrative-light-${sweepNowMs}`, but `buildNarrativeSessionKey` produces keys in the format `dreaming-narrative-${phase}-${workspaceHash}-${nowMs}`. The workspace hash segment is absent, so these calls always target a non-existent key — the safety net silently does nothing. The same mismatch exists for the REM cleanup at line 1669.
To fix, export `buildNarrativeSessionKey` from `dreaming-narrative.ts` (or re-inline the hash logic here) so the defensive cleanup constructs a key that actually matches what was created.
How can I resolve this? If you propose a fix, please make it concise.| const remSessionKey = `dreaming-narrative-rem-${sweepNowMs}`; | ||
| await params.subagent | ||
| .deleteSession({ sessionKey: remSessionKey }) | ||
| .catch(() => { | ||
| // Swallow errors — this is best-effort cleanup. | ||
| }); |
There was a problem hiding this comment.
Same wrong key format for REM cleanup
dreaming-narrative-rem-${sweepNowMs} is missing the workspace hash, so this deleteSession call targets a non-existent key and the cleanup is a no-op, identical to the issue on the light-phase cleanup above.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/memory-core/src/dreaming-phases.ts
Line: 1669-1674
Comment:
**Same wrong key format for REM cleanup**
`dreaming-narrative-rem-${sweepNowMs}` is missing the workspace hash, so this `deleteSession` call targets a non-existent key and the cleanup is a no-op, identical to the issue on the light-phase cleanup above.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9b4b7365
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Defensive cleanup: ensure the light-phase narrative session is deleted even if | ||
| // generateAndAppendDreamNarrative's primary cleanup was skipped due to an error. | ||
| if (params.subagent) { | ||
| const lightSessionKey = `dreaming-narrative-light-${sweepNowMs}`; |
There was a problem hiding this comment.
Use canonical narrative session key in fallback cleanup
The defensive deleteSession key here is constructed as dreaming-narrative-light-${sweepNowMs}, but narrative sessions are created with buildNarrativeSessionKey(...) in dreaming-narrative.ts, which includes a workspace hash (dreaming-narrative-<phase>-<workspaceHash>-<nowMs>). In any run where the primary in-function cleanup does not remove the session (the exact case this block is meant to cover), this fallback will always target the wrong key and leave the leaked session behind; the same mismatch is repeated for REM cleanup.
Useful? React with 👍 / 👎.
b3cd733 to
c11d14f
Compare
3805afb to
88964cc
Compare
…enclaw#66358) Root cause: `runDreamingSweepPhases` passed `params.nowMs` through to each phase without normalizing it first. Each phase recomputed `Date.now()` at call time, producing a slightly different timestamp than the session key used by `generateAndAppendDreamNarrative`. The resulting session key mismatch meant the `deleteSession` call cleaned up a different (or non-existent) key than the one that was created. Fix: - Normalize `nowMs` once at the top of `runDreamingSweepPhases` and pass the consistent value to all phases. - Add a defensive `deleteSession` call in `runDreamingSweepPhases` for each phase after `runLightDreaming`/`runRemDreaming` completes. This acts as a safety net even if the narrative function's primary cleanup is skipped. - Guard the `finally` block in `generateAndAppendDreamNarrative` with `if (params.subagent)` to prevent TypeError if the subagent runtime becomes unavailable mid-flight. This fixes the observed session list pollution where `dreaming-narrative-*-<timestamp>` entries accumulated indefinitely.
88964cc to
51f72b2
Compare
|
Merged via squash.
Thanks @chiyouYCH! |
…66358) (openclaw#67023) Merged via squash. Prepared head SHA: 51f72b2 Co-authored-by: chiyouYCH <26790612+chiyouYCH@users.noreply.github.com> Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com> Reviewed-by: @jalehman
Problem
memory-core creates
dreaming-narrative-*sub-sessions during the Dreaming sweep but they accumulate indefinitely because thedeleteSessioncall in thefinallyblock uses a different session key than what was created.Root Cause
In
runDreamingSweepPhases,params.nowMsis passed through torunLightDreaming/runRemDreamingwithout normalization. Each function recomputesDate.now()independently:The session key written at creation and the key used in
deleteSessioncan differ by milliseconds, so the deletion hits a non-existent key and the session leaks.Fix (2 files)
extensions/memory-core/src/dreaming-phases.ts
nowMsonce at the top ofrunDreamingSweepPhases→sweepNowMssweepNowMsto all phasesdeleteSessioncall after each phase (safety net)extensions/memory-core/src/dreaming-narrative.ts
finallyblock withif (params.subagent)to prevent TypeError if the subagent runtime becomes unavailable mid-flightTesting
Manual trigger of Dreaming sweep on a heavily-polluted system (251 leaking sessions) reduced the count to 0. Subsequent sweeps produce no new leaked sessions.
Full diff: chiyouYCH#1
Files changed:
extensions/memory-core/src/dreaming-phases.ts,extensions/memory-core/src/dreaming-narrative.ts