fix(cli): prevent CLI memory leaks from oversized diffs and session accumulation - #7617
Merged
Conversation
When diffFull() reads file contents via git show, files exceeding 256 KB (e.g. .heapsnapshot JSON) are now treated like binary files — before/after are replaced with empty strings. This prevents multi-GB strings from accumulating in downstream consumers (storage, SSE, TUI, VS Code, sharing).
Defense-in-depth: destructure away before/after content from FileDiff objects at both TUI store entry points (SSE handler + full sync). The sidebar only reads file, additions, deletions — carrying full file content in the Solid store is unnecessary and risks memory bloat.
…content Avoids allocating multi-MB strings in the JS heap for oversized files. Previously the full content was read then discarded; now the object size is checked first via cat-file -s and the git show is skipped entirely when either side exceeds 256 KB.
Contributor
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by gpt-5.4-20260305 · 450,787 tokens |
Existing sessions may have multi-GB before/after strings persisted in session_diff JSON files. The read path in Summary.diff() now checks each entry against the 256 KB cap and replaces oversized content with empty strings, then rewrites the file so subsequent loads are fast. This follows the existing unquoteGitPath migration pattern.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
The Solid store accumulated messages, parts, diffs, todos, status, and permissions for every session visited during a TUI lifetime. Navigating away via /new or the session list never freed the old session's data. Add an evict() function that deletes all per-session entries from the store maps and clears the fullSyncedSessions cache. Wire it into: - A createEffect in app.tsx that fires when the route changes away from a session (on() tracks prev vs current sessionID) - The session.deleted SSE handler, which previously only removed the session list entry but left orphaned per-session data
alex-alecu
changed the title
User messages carry summary.diffs with full before/after file content (the same giant strings as session_diff). The TUI never reads this field. Strip it at both entry points (SSE handler + full sync) to prevent multi-MB strings from accumulating in the Solid store.
Bun's JSC does not return freed native heap pages to the OS within a single Worker lifetime. After large sessions, the only way to reclaim that 2-3 GB of native allocator retention is to terminate the worker and spawn a fresh one. Workaround for oven-sh/bun#28318 - Add getter-indirection layer so fetch/events transparently follow worker replacement without rebuilding the TUI - Add rejectAll() to RPC client to fail in-flight calls on termination - Add rebindable event source that re-registers handlers on new client - Wire /new command to fire-and-forget restart; sync layer re-bootstraps via server.instance.disposed event from the new worker - Guard against re-entry and skip in external server mode
…eclamation Bun Workers are threads within the same OS process — terminate() frees the JSC context but mimalloc retains every page process-wide, so the previous Worker-restart approach had zero effect on RSS. Switch to Bun.spawn() with IPC which creates a separate child process. Killing that process returns all its native memory to the OS. The IPC relay pattern means the RPC client persists across subprocess restarts — event listeners, fetch proxy, and SDK all continue working without getter-indirection or rebinding. Workaround for oven-sh/bun#28318
The subprocess restart approach did not solve the underlying Bun native memory retention issue (oven-sh/bun#28318). Remove all restart plumbing from rpc.ts, thread.ts, worker.ts, app.tsx, and sdk.tsx to keep the diff clean. The diff-size and store-eviction fixes remain.
alex-alecu
force-pushed
the
fix/session-diff-memory-leak
branch
from
March 26, 2026 09:41
048d920 to
5d611d6
Compare
alex-alecu
changed the title
markijbema
reviewed
Mar 26, 2026
| @@ -121,16 +121,22 @@ export namespace SessionSummary { | |||
| }), | |||
| async (input) => { | |||
| const diffs = await Storage.read<Snapshot.FileDiff[]>(["session_diff", input.sessionID]).catch(() => []) | |||
Contributor
There was a problem hiding this comment.
WARNING: Existing oversized session_diff files still load fully before scrub
Storage.read() deserializes the whole JSON blob here, so a previously persisted multi-GB before/after payload is already in memory before the limit check runs. That means the "clean on first load" migration still hits the same first-load memory spike/OOM this PR is trying to prevent; only later reads benefit after the rewrite succeeds.
| if (!agent) throw new Error(`Unknown agent type: ${params.subagent_type} is not a valid agent type`) | ||
|
|
||
| const allowsTask = agent.permission.some((rule) => rule.permission === "task" && rule.action === "allow") | ||
| const allowsTask = agent.permission.some((rule) => rule.permission === "task" && rule.action === "allow") // kilocode_change |
Contributor
There was a problem hiding this comment.
WARNING: Granular task permissions are collapsed to a boolean
This only checks whether the agent has any allowed task rule, then the child session below either gets task: false or no task restriction at all. Rules like "allow explore, deny general" are not persisted into session.permission, so a resumed task session can lose those subagent-specific limits.
| async (input) => { | ||
| const diffs = await Storage.read<Snapshot.FileDiff[]>(["session_diff", input.sessionID]).catch(() => []) | ||
| // kilocode_change start — scrub oversized diffs from stored session_diff | ||
| const limit = 256 * 1024 |
Collaborator
There was a problem hiding this comment.
Why did you not use the constant here?
Contributor
Author
There was a problem hiding this comment.
Fixed in 664ceec. Exported the constant from Snapshot and reused it here.
Contributor
Author
There was a problem hiding this comment.
marius-kilocode
approved these changes
Mar 26, 2026
johnnyeric
approved these changes
Mar 26, 2026
kilo-code-bot Bot
added a commit
that referenced
this pull request
Mar 26, 2026
Research recent changes (Feb 26 - Mar 26, 2026) to tool definitions, system prompts, and agent metadata that could cause performance regressions in long-running sessions by increasing token count. Key findings: - New codebase_search tool (+350 tokens when experimental flag on) - environment_details injection on every request (+15-250 tokens) - Config paths added to system prompt (+35 tokens) - PlanExitTool always registered (filtered by agent permissions) - Session diff memory leak fix (PR #7617) addresses major long-session issue
jliounis
pushed a commit
to jliounis/kilocode
that referenced
this pull request
May 18, 2026
…y-leak fix(cli): prevent CLI memory leaks from oversized diffs and session accumulation
t7tran
pushed a commit
to t7tran/kilocode
that referenced
this pull request
Aug 14, 2026
…y-leak fix(cli): prevent CLI memory leaks from oversized diffs and session accumulation
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
4 participants
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.
Why
Long-running CLI sessions could consume over 1.5 GB of memory for two reasons: huge files (like heap snapshots) were read in full and stored as diff content, and switching sessions via
/newnever freed the old session's data from the TUI store.What changed
Diff size guard and cleanup
The diff engine now checks each file's byte size before reading it. If either the before or after version exceeds 256 KB, the content is skipped entirely — just like binary files are already handled. This stops oversized strings from ever entering memory. Existing sessions that already have oversized diffs stored on disk are cleaned on first load — the server-side read path detects and strips oversized content, then rewrites the file so subsequent loads are fast.
TUI store memory management
The TUI store now strips large file content at every entry point.
session_diffentries havebefore/afterremoved on arrival (the sidebar only needs filenames and counts). User messages havesummary.diffsemptied since the TUI never reads that field — this was a second path for the same giant strings to enter memory.The TUI store also evicts all per-session data (messages, parts, diffs, todos, status) when navigating away from a session. Previously this data accumulated for every session visited during a TUI lifetime. The
session.deletedhandler also cleans up orphaned per-session data.Known limitation: native memory retention after
/newAfter typing
/new, the JS heap is clean — heap snapshots confirm ~103 MB with zero retained session data. However, process RSS reported by the OS may remain elevated because Bun's memory allocator (JavaScriptCore + mimalloc) does not return freed virtual memory pages to the OS within a single process lifetime. This is a known upstream issue: oven-sh/bun#28318.In practice, most sessions should be fine — the fixes in this PR dramatically reduce peak memory by preventing oversized strings from entering the TUI store in the first place, and evicting per-session data on navigation. The native allocator retention only becomes noticeable after very large sessions (multi-GB working sets), and restarting the CLI fully reclaims all memory.
How to test
/newto start a fresh session.