Skip to content

fix(chat): reap replaced stream buffers#3825

Open
CompleteDotTech wants to merge 1 commit into
mainfrom
agent/fix-heap-retention-3803
Open

fix(chat): reap replaced stream buffers#3825
CompleteDotTech wants to merge 1 commit into
mainfrom
agent/fix-heap-retention-3803

Conversation

@CompleteDotTech

Copy link
Copy Markdown
Member

Summary

Fixes #3803.

A finished live-chat resume buffer could become permanently reachable when a follow-up turn reused the same conversation ID. openRunBuffer() replaced the shared conversation mapping but cleared the predecessor's two-minute reap timer, even though the predecessor remained mapped by its unique run ID.

The fix keeps the predecessor timer armed. Its existing identity guard removes only mappings that still point to that predecessor, so it cannot delete the replacement conversation mapping.

Bead: cave-r13x

Measured root cause

No in-the-wild snapshot was present on this Windows machine, so the live-/chat/send suspect was reproduced directly with 300 finished runs sharing one conversation, each filling the bounded 512 KiB resume ring.

Before the fix, after GC:

  • heap used: 7.8 MiB → 136.2 MiB (+128.4 MiB)
  • old run IDs still reachable: 300 / 300
  • retained-size ranking: (string) 136.817 MiB, Object 136.037 MiB, Map 132.015 MiB
  • dominant Map instance retained 131.292 MiB

The retainer path from the V8 snapshot was:

chat-stream-buffer module Context → buffers Map → RunBuffer → events Array → serialized StreamEvent JSON strings

After the fix and the virtual two-minute retention window:

  • heap used: 140.2 MiB → 9.3 MiB
  • released: 130.8 MiB
  • old run IDs still reachable: 0 / 300
  • the fixed snapshot's class ranking returned near the baseline snapshot

Regression coverage

The new test finishes an old run, replaces its shared conversation key with a new run, advances the two-minute timer, and proves:

  • the predecessor disappears under its unique run ID
  • the replacement remains reachable under the shared conversation key

The regression failed before the implementation change and passes after it.

Verification

Passed:

  • chat stream buffer: 10 / 10
  • chat stream route: 5 / 5
  • chat stream status route: 6 / 6
  • API route contracts: 232
  • pnpm typecheck
  • pnpm lint
  • pnpm check:tests-wired (1,234 test files wired)
  • git diff --check

Broader local runners encountered unrelated Windows harness assumptions:

  • pnpm test:app stopped after 74 files because src-tauri/release-runtime.test.mjs expects LF-only Rust source while this checkout uses CRLF
  • pnpm test:api stopped after 6 files because scripts/git-hooks-pre-commit.test.mjs requires /bin/bash, which is unavailable on this machine

Remaining uncertainty

This is a controlled reproduction rather than the originally requested in-the-wild capture because no production snapshot or [heap-monitor] warning existed. The reproduction exercises the prime live-chat suspect directly, produces the expected unbounded per-turn retention, identifies the retaining path in a V8 heap snapshot, and demonstrates bounded post-fix behavior.

Copy link
Copy Markdown
Member Author

@BunsDev This PR is ready to merge after testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyze first in-the-wild heap snapshot from the heap monitor

2 participants