Skip to content

fix(#288): derive-and-drop sub-agent state (stop retaining raw entries — 85% of heap) #322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Juliusolsson05 merged 2 commits into from
Jun 21, 2026
Merged

fix(#288): derive-and-drop sub-agent state (stop retaining raw entries — 85% of heap) #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14c19b6
fix(#288): derive-and-drop sub-agent state — stop retaining raw entri…
Juliusolsson05 Jun 21, 2026
dff002d
fix(#322): flatten retained tool-call headlines (V8 slice-trap) + bou…
Juliusolsson05 Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/main/subagents/SubAgentWatcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ afterEach(async () => {
})

describe('SubAgentWatcher', () => {
it('buffers partial JSONL lines and caps retained parsed entries', async () => {
it('buffers partial JSONL lines and folds every entry without retaining them', async () => {
const dir = await mkdtemp(join(tmpdir(), 'agent-code-subagents-'))
tmpDirs.push(dir)
const jsonlPath = join(dir, 'agent-child.jsonl')
Expand Down Expand Up @@ -49,12 +49,17 @@ describe('SubAgentWatcher', () => {
// WHY 520 entries instead of a tiny fixture:
//
// The renderer-facing builder already caps the mini-feed to 40 tool
// calls. To prove the watcher now bounds MAIN-process retained RawEntry
// memory too, the fixture must exceed the watcher cap (500), not only the
// renderer cap. Without the watcher cap this would report 481 dropped
// tool calls (521 total - 40 visible); with the watcher cap it reports
// 460 (500 retained - 40 visible), proving older parsed entries were
// discarded before building state.
// calls. This large fixture proves the watcher's #288 derive-and-drop
// accumulator folds EVERY appended entry (no raw entries retained) while
// still bounding what it ships to the renderer.
//
// CONSCIOUS BEHAVIOR DELTA (#288): the old watcher retained at most 500
// raw entries and folded only that 500-entry tail, so `droppedToolCalls`
// was relative-to-500 and this asserted 460 (500 retained - 40 visible).
// The accumulator folds from the start, so `droppedToolCalls` is now the
// TRUE total: 521 tool_uses - 40 visible = 481. That is more correct (the
// "+N earlier" count reflects the real number dropped, not an artifact of
// a retention cap), and it no longer depends on retaining any entries.
await appendFile(
jsonlPath,
Array.from({ length: 520 }, (_, i) => assistantToolUseLine(i + 1)).join(''),
Expand All @@ -65,7 +70,7 @@ describe('SubAgentWatcher', () => {
await eventually(() => {
const state = emissions.at(-1)?.['tool-parent']
expect(state?.toolCalls).toHaveLength(40)
expect(state?.droppedToolCalls).toBe(460)
expect(state?.droppedToolCalls).toBe(481)
expect(state?.toolCalls.at(-1)).toMatchObject({
name: 'Read',
headline: '/tmp/file-520.txt',
Expand Down
238 changes: 62 additions & 176 deletions src/main/subagents/SubAgentWatcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ import { createReadStream } from 'node:fs'
import { readFile, readdir, stat } from 'node:fs/promises'
import { join } from 'node:path'
import type { SubAgentState } from '@preload/api/types.js'
import { buildSubAgentState, type SubAgentMeta } from './subagentState.js'
import { makeStringPool, internEntryFields } from '@main/sessions/internEntry.js'
import {
accumulateSubAgentEntry,
buildSubAgentStateFromAccumulator,
createAccumulator,
type SubAgentAccumulator,
type SubAgentMeta,
} from './subagentState.js'

// One poller per session, watching <sessionDir>/subagents/.
//
Expand All @@ -15,160 +20,49 @@ import { makeStringPool, internEntryFields } from '@main/sessions/internEntry.js
// something actually changed (dirty flag) to avoid spamming IPC.

const POLL_MS = 600
const MAX_RETAINED_ENTRIES_PER_AGENT = 500

// #288 (root cause): cap the BYTE SIZE of every retained free-text field, not
// just the entry COUNT.
// #288 ROOT-CAUSE FIX: this watcher no longer retains raw entries at all.
//
// A dominator-tree heap analysis (scripts/analyze-heapsnapshot.mjs --owners)
// proved that 7 live SubAgentWatcher instances retained 263 MB — 88% of the
// entire reachable main-process heap — via the chain
// HISTORY (why the old code was the way it was, so future-you doesn't reinvent
// it): a dominator-tree heap analysis (scripts/analyze-heapsnapshot.mjs --owners)
// pinned live SubAgentWatcher instances as ~85% of a 227 MB main-process heap via
// SubAgentWatcher.entriesByAgent (Map<agentId, RawEntry[]>)
// → entry → message.content → huge string.
// PR #300 capped MAX_RETAINED_ENTRIES_PER_AGENT=500 but did NOT bound the bytes
// per entry, so each of those 500 retained entries still pinned its FULL body:
// Read tool outputs, tool_result bodies, the skills-list attachment, and
// `[Truncated: PARTIAL view]` system-reminders. 500 entries × multi-MB bodies ×
// several agents × 7 watchers = 263 MB, and churning those multi-MB strings on
// every 600 ms poll fragments V8 into ~1.5 GB RSS.
// We chased that with COUNT caps (PR #300: keep 500 entries) and then per-FIELD
// byte caps (PR #320/#321: clamp every string in each retained entry). Both only
// SHRANK the retained entries; they never questioned WHY we retained entries at
// all. The answer is: we didn't need to. buildSubAgentState was a pure fold over
// the entries, re-run on every emit, so the only reason to keep entries was to
// re-fold them. By folding INCREMENTALLY as each line streams in (see
// accumulateSubAgentEntry in subagentState.ts) and dropping the entry, the
// O(transcript) retention disappears entirely — no count cap, no byte cap, no
// string pool, no truncation. The durable full transcript still lives on disk in
// agent-<id>.jsonl. This mirrors the Codex twin (codexSubagentState.ts, PR #317).
//
// The mini-feed this watcher feeds is a LIVENESS affordance, not a transcript
// archive. buildSubAgentState only reads structure (block types, tool names,
// tool_use_id, is_error, timestamps) plus an ≤80-char headline per tool call —
// it never renders a full body. So truncating the large free-text payloads at
// retention to a cap well above what the renderer shows is invisible to the
// user while removing the bytes that caused the leak. The durable full
// transcript always stays on disk in the agent-<id>.jsonl, which is the source
// of truth if a future UI wants deep drill-in.
//
// 8 KB is generous: it is ~100× the 80-char headline buildSubAgentState renders
// from any single field, so no rendered value can ever be clipped by this cap,
// yet it turns a multi-MB Read result into a few KB. We truncate per-field
// (not per-entry) so an entry with several blocks each keeps its own budget.
//
// NOTE on units: this is a UTF-16 *character* cap (`String.length` counts code
// units, not bytes) — the truncation marker says "chars" to match. We don't
// convert to bytes because the goal is a generous BOUND, not an exact size, and
// a char count is the cheapest thing to compute on this per-appended-line path.
const MAX_ENTRY_FIELD_BYTES = 8 * 1024

// truncateEntryBodies: clamp the large free-text string fields of a freshly
// parsed RawEntry in place, PRESERVING every structural field buildSubAgentState
// reads (entry `type`/`uuid`, `message.role`/`type`, each block's `type`, tool
// `name`/`id`/`tool_use_id`, `is_error`/status, timestamps). We only shrink the
// payloads that carry the megabytes:
// - TOP-LEVEL `entry.content` string and `entry.attachment.content` string.
// Some entries have NO `message` field at all — most importantly the
// `skill_listing` attachment whose `content` is the full skills list. That
// shape was the single LARGEST duplicated leak category in the heap (×803).
// It is handled at the entry root, BEFORE/independent of the message walk,
// because there is no `message.content` array to descend into — the old
// early-return ("not an array") skipped these entries entirely and made the
// per-block `attachment.content` clamp below dead code for the real shape.
// - `message.content` when it is a plain STRING. Claude transcripts use a
// string body (large pasted user prompts, compact/user string messages,
// string-shaped system reminders) as readily as the array-of-blocks shape;
// the old early-return pinned every string body full-size.
// - text/thinking blocks' `text`/`thinking`
// - tool_result blocks' `content`/`output` (string OR array-of-{text} shape)
// - string values inside a tool_use `input`, INCLUDING NESTED ones — e.g.
// MultiEdit's `input.edits[].old_string`/`new_string`, which a flat
// top-level-only walk left pinned. We recurse (bounded) so nested text is
// clamped while every key and the input's structure stay intact.
// - a per-block `attachment.content` (skills lists can also ride inside blocks)
// Each clamp appends an honest marker so a future reader of the in-memory entry
// knows chars were dropped and where the full body lives. This runs once per
// appended line at the single parse/push site below — never on a hot render
// path — so the cost is bounded by new bytes, matching the watcher's invariant.
function clampString(value: string, cap: number): string {
if (value.length <= cap) return value
const dropped = value.length - cap
// CRITICAL (#288): `value.slice(0, cap)` returns a V8 SlicedString that RETAINS
// the entire parent string — so truncating with a bare slice frees ZERO memory
// (the multi-MB body we are trying to drop stays alive behind the slice, which
// is exactly why #320 reduced the heap by nothing). Proven empirically: clamping
// 2000×200KB strings via slice left ~194MB pinned; via the Buffer round-trip
// below, ~19MB. Round-tripping the prefix through a Buffer copies its bytes into
// fresh, parent-independent storage, so the original body becomes GC-eligible.
// `utf16le` preserves every JS code unit exactly (incl. lone surrogates), so the
// visible prefix is byte-identical to a plain slice — only the retention differs.
const head = Buffer.from(value.slice(0, cap), 'utf16le').toString('utf16le')
return head + `… [truncated ${dropped} chars — full body on disk]`
}

// clampDeep: recursively clamp every string VALUE reachable inside an arbitrary
// tool_use `input`, preserving all keys, array order, and object structure.
// WHY recursion (not the old flat top-level-only loop): structured tool inputs
// bury their large text below the first level — MultiEdit puts each edit's
// `old_string`/`new_string` under `input.edits[i]`, and other tools nest text
// under sub-objects/arrays. A flat walk only saw the top-level string values
// and left those nested megabytes pinned. We bound recursion at `depth` (≤6) so
// a pathologically deep object can't blow the stack or burn the per-line budget;
// nothing deeper than that is a realistic tool-input text payload, and the
// durable full copy is on disk regardless. Only string VALUES are rewritten —
// keys and the shape headlineFromInput reads (command/file_path/…) are untouched.
function clampDeep(value: unknown, cap: number, depth: number): unknown {
if (typeof value === 'string') return clampString(value, cap)
if (depth <= 0 || !value || typeof value !== 'object') return value
if (Array.isArray(value)) {
for (let i = 0; i < value.length; i++) value[i] = clampDeep(value[i], cap, depth - 1)
return value
}
const obj = value as Record<string, unknown>
for (const k of Object.keys(obj)) obj[k] = clampDeep(obj[k], cap, depth - 1)
return obj
}

// Deepest nesting we recurse when clamping an entry. The big bodies sit shallow
// (`entry.toolUseResult.file.content` ≈ depth 3; `message.content[i].content[j]
// .text` ≈ depth 6); 12 is generous headroom while still bounding the walk so a
// pathological object can't blow the stack or burn the per-line budget.
const ENTRY_CLAMP_DEPTH = 12

function truncateEntryBodies(entry: unknown, cap: number): void {
// #288 (fix for the #320 miss): clamp EVERY large free-text string ANYWHERE in
// the entry by recursing the whole object — not a hand-maintained list of fields.
//
// WHY the previous field-by-field walk was wrong: it only visited
// `message.content` / `attachment` / bare `content`, but Claude writes the FULL
// Read/tool output to the TOP-LEVEL `toolUseResult` field
// (`toolUseResult.file.content`, `stdout`, `stderr`) — *in addition* to the
// tool_result block. The dominator tree's bytes actually lived in
// `toolUseResult`, which the targeted walk never touched, so retained entries
// stayed full-size and #320 freed almost nothing. A recursive clamp catches
// toolUseResult, message.content (string OR array), attachments, tool_use input,
// and any future shape — no more whack-a-mole.
//
// WHY it's safe / invisible: `clampString` is a no-op for strings under `cap`,
// so short structural fields (type/uuid/role/name/id/tool_use_id/timestamps) are
// returned untouched; only multi-KB bodies are clamped. `buildSubAgentState`
// only ever renders ≤80-char headlines, so clamping at `cap` (8 KB) can never
// change a rendered value. `clampDeep` rewrites string VALUES in place and
// preserves all keys and array order. Durable full bodies stay on disk.
clampDeep(entry, cap, ENTRY_CLAMP_DEPTH)
}
// What that deletes from THIS file: truncateEntryBodies + clampDeep + clampString
// (the #320/#321 byte-cap machinery) and the internEntryFields/makeStringPool
// usage — all of it existed solely to make the retained entries cheaper, and with
// nothing retained there is nothing to clamp or intern. internEntryFields itself
// stays in internEntry.ts (jsonlCoalescer + historyLoader still use it).

type ParentResult = (toolUseId: string) => { done: boolean; error: boolean }

type RawEntry = Parameters<typeof buildSubAgentState>[3][number]
// The accumulator's fold consumes the same RawEntry shape buildSubAgentState did.
type RawEntry = Parameters<typeof accumulateSubAgentEntry>[1]

export class SubAgentWatcher {
private timer: NodeJS.Timeout | null = null
private offsets = new Map<string, number>() // agentId -> byte offset consumed
private partialByAgent = new Map<string, string>()
private entriesByAgent = new Map<string, RawEntry[]>()
// #288 ROOT-CAUSE FIX: per-agent ACCUMULATOR instead of a retained entry array.
// Each appended line is folded into its agent's accumulator (a few scalars + a
// bounded ≤60 tool-call ring) and then dropped. Memory is O(open tool calls),
// not O(transcript length) — which is what killed the heap before. emit() reads
// the live accumulator each call so a late parentResult flip still re-renders.
private accByAgent = new Map<string, SubAgentAccumulator>()
private metaByAgent = new Map<string, SubAgentMeta>()
private dirty = false
private stopped = false
// #288: per-watcher string pool. This watcher retains up to
// MAX_RETAINED_ENTRIES_PER_AGENT parsed RawEntry objects PER subagent
// across a whole Task fan-out, each freshly JSON.parsed below, so the
// same cwd/role/type metadata is re-minted on every appended line.
// Interning against a pool scoped to this watcher shares the canonical
// strings across all of its agents' entries. The pool's lifetime is the
// watcher's — cleared in stop() (one watcher per session) — so it can
// never grow into the global leak a shared pool would be.
private intern = makeStringPool()

constructor(
private readonly subagentsDir: string,
Expand All @@ -190,12 +84,10 @@ export class SubAgentWatcher {
this.timer = null
this.offsets.clear()
this.partialByAgent.clear()
this.entriesByAgent.clear()
// #288: drop the per-agent accumulators (each holds only scalars + a small
// bounded ring, but a stopped watcher must release everything regardless).
this.accByAgent.clear()
this.metaByAgent.clear()
// #288: drop the interning pool's backing Map so the strings it pinned
// are released with the rest of this watcher's state. Reassigning a
// fresh empty pool is the cheapest way to let the old Map GC.
this.intern = makeStringPool()
}

/** Force a re-emit (e.g. the parent transcript just produced a tool_result
Expand Down Expand Up @@ -266,43 +158,31 @@ export class SubAgentWatcher {
const complete = text.slice(0, lastNl)
this.partialByAgent.set(agentId, text.slice(lastNl + 1))
this.offsets.set(agentId, size)
const arr = this.entriesByAgent.get(agentId) ?? []
// #288 ROOT-CAUSE FIX: fold each appended line into the agent's accumulator
// and let the parsed entry die. There is deliberately NO retained array, NO
// 500-cap splice, NO truncate, NO intern here — nothing entry-shaped survives
// a loop iteration, so the multi-MB tool-result/Read bodies the dominator
// tree pinned can never accumulate. accumulateSubAgentEntry runs the SAME
// per-entry logic buildSubAgentState's loop did; the entry is read-only and
// becomes GC-eligible the instant the iteration ends.
let acc = this.accByAgent.get(agentId)
if (!acc) {
acc = createAccumulator()
this.accByAgent.set(agentId, acc)
}
for (const line of complete.split('\n')) {
const t = line.trim()
if (!t) continue
try {
const parsed = JSON.parse(t) as RawEntry
// #288 (root cause): clamp the large free-text bodies BEFORE retaining,
// so the 263 MB the dominator tree attributed to entriesByAgent → entry
// → message.content → huge string can never accumulate. Done here, at the
// single parse/push site, the cap applies to every entry exactly once and
// the durable full body stays on disk. See truncateEntryBodies above.
truncateEntryBodies(parsed, MAX_ENTRY_FIELD_BYTES)
// #288: intern the duplicated metadata (type, message.role/type, …)
// before retaining. internEntryFields is defensive and never throws,
// so a malformed entry that slipped past JSON.parse still can't take
// down this loop. Value-equality is preserved — buildSubAgentState
// reads the same fields it always did.
internEntryFields(parsed as Record<string, unknown>, this.intern)
arr.push(parsed)
// accumulateSubAgentEntry is defensive (permissive shapes, no throws on a
// partial/edge entry), matching the old builder's contract. A malformed
// line that slips past JSON.parse still can't take down this loop.
accumulateSubAgentEntry(acc, parsed)
} catch {
/* skip a malformed line */
}
}
if (arr.length > MAX_RETAINED_ENTRIES_PER_AGENT) {
// WHY cap parsed entries even though the renderer timeline is already
// capped in buildSubAgentState:
//
// The IPC payload cap only protects the renderer. Keeping every parsed
// RawEntry here still makes main-process heap grow with long-running
// subagents and duplicates large tool-result strings in memory. The
// mini-feed is a liveness/status affordance, not a full transcript
// archive; the durable source remains the on-disk JSONL if a future UI
// wants deep drill-in. Retaining a generous tail preserves current
// activity and recent tool context while bounding this watcher.
arr.splice(0, arr.length - MAX_RETAINED_ENTRIES_PER_AGENT)
}
this.entriesByAgent.set(agentId, arr)
this.dirty = true
}

Expand All @@ -311,13 +191,19 @@ export class SubAgentWatcher {
for (const [agentId, meta] of this.metaByAgent) {
const toolUseId = meta.toolUseId
if (!toolUseId) continue // can't link without it — skip (design spec §8)
const entries = this.entriesByAgent.get(agentId) ?? []
// INVARIANT 6: read the LIVE accumulator each emit and resolve terminal
// done/error here at build time. A late parentResult flip (parent
// tool_result lands after the subagent's last line) therefore re-renders on
// the next emit without any state stored in the accumulator. An agent whose
// meta exists but whose jsonl hasn't been read yet folds an empty
// accumulator → the same "header only" state the old empty-array path gave.
const acc = this.accByAgent.get(agentId) ?? createAccumulator()
const { done, error } = this.parentResult(toolUseId)
out[toolUseId] = buildSubAgentState(
out[toolUseId] = buildSubAgentStateFromAccumulator(
acc,
toolUseId,
agentId,
meta,
entries,
done,
error,
)
Expand Down
Loading